[Settings] Split up hotspot.js

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

({perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [c= p=3 s=2013.11.08 u=])

Attachments

(1 attachment, 1 obsolete attachment)

46 bytes, text/x-github-pull-request
arthurcc
: review+
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
We should split up this panel so that we do not load subpanels as dependencies when we load the panel.
(Assignee)

Comment 1

5 years ago
Created attachment 821520 [details] [review]
Github pull request
(Assignee)

Comment 2

5 years ago
Note: this requires bug 929813 to land first.
(Assignee)

Updated

5 years ago
Attachment #821520 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
Created attachment 822609 [details] [review]
Github pull request
(Assignee)

Comment 4

5 years ago
Comment on attachment 822609 [details] [review]
Github pull request

Arthur - here's one more panel down if you have the time to review. Thanks!
Attachment #822609 - Flags: review?(arthur.chen)
Comment on attachment 822609 [details] [review]
Github pull request

Sorry for the late review. Please refer to my github comments, thanks!
Attachment #822609 - Flags: review?(arthur.chen)
(Assignee)

Comment 6

5 years ago
Comment on attachment 822609 [details] [review]
Github pull request

Hey Arthur - Great comments on the pull request. I've tried to address them, and remove some duplicated code between the init() and reset() functions. Let me know what you think. Thanks!
Attachment #822609 - Flags: review?(arthur.chen)
Comment on attachment 822609 [details] [review]
Github pull request

What a prompt revision! With this patch the panel is not restoring the settings, please check my comments. :)
Attachment #822609 - Flags: review?(arthur.chen)
(Assignee)

Comment 8

5 years ago
Comment on attachment 822609 [details] [review]
Github pull request

Hi Arthur,

Thanks for the comments. I've fixed the issue, and added a marionette test so we don't regress in the future. Please review when you have time.
Attachment #822609 - Flags: review?(arthur.chen)
Comment on attachment 822609 [details] [review]
Github pull request

r=me. Thank you for the effort!
Attachment #822609 - Flags: review?(arthur.chen) → review+
FYI, it looks like there is a settings failure in travis.  Might want to double check that before landing.
(Assignee)

Comment 11

5 years ago
(In reply to Ben Kelly [:bkelly] from comment #10)
> FYI, it looks like there is a settings failure in travis.  Might want to
> double check that before landing.

Thanks, will definitely ensure green-ness before merging.
(Assignee)

Comment 12

5 years ago
Landed in master: https://github.com/mozilla-b2g/gaia/commit/b27c0249c5877f6a55a8de37f0eef8770b515711
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [c= p=3 s= u=] → [c= p=3 s=2013.11.08 u=]
You need to log in before you can comment on or make changes to this bug.