Closed
Bug 1223280
Opened 9 years ago
Closed 9 years ago
Migrate network-alerts to use NotificationHelper api
Categories
(Firefox OS Graveyard :: Gaia::Network Alerts, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: heidi.kasemir, Assigned: heidi.kasemir, Mentored)
References
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36
Comment 1•9 years ago
|
||
Updated•9 years ago
|
Assignee: nobody → heidi.kasemir
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: migrate network-alerts to use NotificationHelper api → Migrate network-alerts to use NotificationHelper api
Assignee | ||
Updated•9 years ago
|
Component: General → Gaia::Network Alerts
Assignee | ||
Updated•9 years ago
|
Attachment #8685288 -
Flags: review?(schung)
Updated•9 years ago
|
Mentor: gandalf
Comment 2•9 years ago
|
||
Some comments on github, please ping me when you finished.
Hi gandalf, do you have anything to add about the patch?
Flags: needinfo?(gandalf)
Comment 3•9 years ago
|
||
Comment on attachment 8685288 [details] [review]
[gaia] hkasemir:1223280-network-alerts-notificationHelper > mozilla-b2g:master
Hey heidi, please just request the review again once you are ready, thanks!
Attachment #8685288 -
Flags: review?(schung)
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8685288 [details] [review]
[gaia] hkasemir:1223280-network-alerts-notificationHelper > mozilla-b2g:master
Comments addressed except for one suggested clean up that became more complex than originally thought due to the existing functionality of the code. Ready for another review!
Attachment #8685288 -
Flags: review?(schung)
Comment 5•9 years ago
|
||
(In reply to heidi.kasemir from comment #4)
> Comment on attachment 8685288 [details] [review]
> [gaia] hkasemir:1223280-network-alerts-notificationHelper >
> mozilla-b2g:master
>
> Comments addressed except for one suggested clean up that became more
> complex than originally thought due to the existing functionality of the
> code. Ready for another review!
I left some few comments on the github. It should be good enough except the comments, so please squash the commits once you address the rest of issues, thanks!
Comment 6•9 years ago
|
||
Comment on attachment 8685288 [details] [review]
[gaia] hkasemir:1223280-network-alerts-notificationHelper > mozilla-b2g:master
And please fix the conflicts as well, thanks!
Attachment #8685288 -
Flags: review?(schung)
Comment 7•9 years ago
|
||
Comment on attachment 8685288 [details] [review]
[gaia] hkasemir:1223280-network-alerts-notificationHelper > mozilla-b2g:master
Steve, Heidi cleaned up the patch according to your feedback. Can you review it?
Flags: needinfo?(gandalf)
Attachment #8685288 -
Flags: review?(schung)
Comment 8•9 years ago
|
||
Comment on attachment 8685288 [details] [review]
[gaia] hkasemir:1223280-network-alerts-notificationHelper > mozilla-b2g:master
Looks good, thanks for the contribution!
Attachment #8685288 -
Flags: review?(schung) → review+
Comment 9•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•