1223275 - Migrate email app to NotificationHelper

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[heidi.kasemir]

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

[GitHub Autolander Bot]

Attachment: [gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master

Comment 2

[James Burke [:jrburke]]

Putting this in the Gaia::Email component for better tracking. 

If you are ready for the pull request to be reviewed, you can click on the "Details" link for the attachment, and set the "review" flag to "?" and specify a reviewer. Since these changes are in the email app, you can set me as the reviewer.

For other parts of Gaia, this link lists who can review the different parts:
https://wiki.mozilla.org/Modules/FirefoxOS

but the "suggested reviewers" link next to the review "?" flag option should also list possible reviewers.

If you are still putting the final touches on the change but have questions, I can help answer them.

Comment 3

[heidi.kasemir]

Comment on attachment 8685278 [details] [review]
[gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master

Thanks in advance for reviewing! I haven't touched the tests, so I'm not sure if those are all going to turn out as desired, but let me know what updates I can make.

Comment 4

[James Burke [:jrburke]]

Comment on attachment 8685278 [details] [review]
[gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master

Thanks for jumping in to fix this bug! It is looking really close, there is an async issue we should sort out, and one of the bodyL10n assignments should switch to the raw: structure, comments left in the pull request.

Flip the review back to "?" to me once you are done with the changes.

As for the tests, the ones that are active for email will be tested by the "https://treeherder.mozilla.org" links that "autolander" puts in the pull request, so you can get an idea if it passes.

For this particular change though, I do not expect any changes are needed for the tests since it is an internal structure change for the notifications, the observable end result should be the same as before.

Comment 5

[heidi.kasemir]

Comment on attachment 8685278 [details] [review]
[gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master

Thanks for the comments, and good solution on the async problem! Let me know if there are any other issues!

Comment 6

[James Burke [:jrburke]]

Comment on attachment 8685278 [details] [review]
[gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master

Looks good, thank you very much for improving the email app! I will merge with master shortly.

Comment 7

[James Burke [:jrburke]]

Merged in master:
https://github.com/mozilla-b2g/gaia/commit/35e1e8e2fdd35ebc4e3b9869c3d4ffb4bcc03461

from pull request:
https://github.com/mozilla-b2g/gaia/pull/33099