Closed Bug 1087981 Opened 10 years ago Closed 9 years ago

[Message] Lazy init settings in notify.js

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::SMS
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

Status:
RESOLVED FIXED
Milestone:
2.2 S5 (6feb)
Project Flags:
blocking-b2g 2.2+
Tracking Flags:
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: julienw, Assigned: julienw)

References

Details

(Whiteboard: [p=1])

Attachments

(1 file)

github PR
46 bytes, text/x-github-pull-request
steveck
: review+
steveck
: feedback+
bajaj
: approval-gaia-v2.2+
Details | Review 
      No description provided.
Attached file github PR — 
I haven't tested the patch on a device yet.

I removed the observer because in this specific case we don't need to be really quick.
Assignee: nobody → felash
Comment on attachment 8510264 [details] [review]
github PR

Works fine for me :)

Steve, what do you think?
Attachment #8510264 - Flags: review?(schung)
Comment on attachment 8510264 [details] [review]
github PR

I'm fine if you prefer less modification in order to uplift it. I think we just need additional assertion for vibrate when document is not hidden. Just ping me when the test is ready, thanks!
Attachment #8510264 - Flags: review?(schung)
We should consider making it a feature blocker if this improves launch time.
feature-b2g: --- → 2.2?
I'm marking feature-b2g:2.2+, as it's blocking performance meta bug.
If it turns out the approach doesn't improve, then just close the bug or remove flag.
feature-b2g: 2.2? → 2.2+
Status: NEW → ASSIGNED
Blocks: sms-sprint-2.2S4
Whiteboard: [p=1]
Target Milestone: --- → 2.2 S4 (23jan)
[changing flag for the performance improvement task]
blocking-b2g: --- → 2.2+
feature-b2g: 2.2+ → ---
Comment on attachment 8510264 [details] [review]
github PR

Hey Steve, I think it's a ready for a second review :) hopefully the final one ;)
Attachment #8510264 - Flags: review?(schung)
Comment on attachment 8510264 [details] [review]
github PR

Only some small suggestions on github, and looks good overall, thanks!
Attachment #8510264 - Flags: review?(schung) → review+
Comment on attachment 8510264 [details] [review]
github PR

Hey Steve, can you have a last look at the last commit? I replaced the settings name by variables. I'm not so sure it's better though, so I'd like your advice.
Attachment #8510264 - Flags: feedback?(schung)
Comment on attachment 8510264 [details] [review]
github PR

Yes, that's what I thought, thanks!
Attachment #8510264 - Flags: feedback?(schung) → feedback+
master: 0368004d46ae09c3e3694d087da3c7c37d27139c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8510264 [details] [review]
github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): -
[User impact] if declined: slightly less performance at startup
[Testing completed]: yes / manual and unit tests
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Note that this is not in the critical path for launching the SMS app, so maybe we should not uplift this. That said, it's a fairly safe patch.
Attachment #8510264 - Flags: approval-gaia-v2.2?
status-b2g-v2.2: --- → affected
status-b2g-master: --- → fixed
Target Milestone: 2.2 S4 (23jan) → 2.2 S5 (6feb)
Attachment #8510264 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: