Migrate email app to NotificationHelper

RESOLVED FIXED

Status

Firefox OS
Gaia::E-Mail
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: heidi.kasemir, Assigned: heidi.kasemir, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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
Created attachment 8685278 [details] [review]
[gaia] hkasemir:1223275-email-notificationHelper > mozilla-b2g:master
(Assignee)

Updated

2 years ago
Blocks: 1111841
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.
Component: General → Gaia::E-Mail
Assignee: nobody → heidi.kasemir
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Summary: migrate email app to NotificationHelper → Migrate email app to NotificationHelper
(Assignee)

Comment 3

2 years ago
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.
Attachment #8685278 - Flags: review?(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.
Attachment #8685278 - Flags: review?(jrburke)
(Assignee)

Comment 5

2 years ago
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!
Attachment #8685278 - Flags: review?(jrburke)
Mentor: gandalf@aviary.pl
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.
Attachment #8685278 - Flags: review?(jrburke) → review+
Merged in master:
https://github.com/mozilla-b2g/gaia/commit/35e1e8e2fdd35ebc4e3b9869c3d4ffb4bcc03461

from pull request:
https://github.com/mozilla-b2g/gaia/pull/33099
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Updated

2 years ago
Blocks: 1225122
You need to log in before you can comment on or make changes to this bug.