1078128 - [Settings] remove try_show_homescreen_section.js and move this logic into root.js

Status: RESOLVED FIXED

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

Description

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

In order to make code clean and robust, it would be better to move try_show_homescreen_scetion.js logic into root.js (with AMD style).

Comment 1

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: patch on master

Comment 2

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8500834 [details] [review]
patch on master

Arthur,

in this patch I did remove try_* script and added an important manager - AppsManager. With this, we can easily fetch out apps from cache and this would make the performance better I think !

I will keep adding the unit tests and please help me check whether this implementation is okay to you or not.

Any feedback / idea would be really appreciated and welcome.

Thanks !!

Comment 3

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Just rebased to latest master and will file a following bug to remove redundant calls to mgmt.

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8500834 [details] [review]
patch on master

Thanks for the patch! f=me.

Comment 5

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8500834 [details] [review]
patch on master

Arthur,

I addressed all your comments just now and also update tests, so I think we are good to have a proper review process now !

Thanks !

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8500834 [details] [review]
patch on master

The original implementation delay the query of all apps so I think we should keep that. And there are some comments regarding the async function design, please check them in github, thanks!

Comment 7

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8500834 [details] [review]
patch on master

Arthur, just updated the patch and I think it looks better now, please help me review again to see whether we are on the right way or not. 

Thanks !!

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8500834 [details] [review]
patch on master

Please check my comments regarding the init function in github, thanks.

Comment 9

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8500834 [details] [review]
patch on master

Nice, with this new update, I think appsCache is more reliable and stable now. I also updated all test cases with new design and did manual test already, Arthur, please help to find some time review this part. Thanks !!

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8500834 [details] [review]
patch on master

Please check my comments regarding tests, thanks.

Comment 11

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8500834 [details] [review]
patch on master

Thanks Arthur, all comments were addressed !

Comment 12

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

+ manual tests already

Comment 13

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8500834 [details] [review]
patch on master

Awesome! r=me, thanks.

Comment 14

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Merged into Gaia/master (2.2): https://github.com/mozilla-b2g/gaia/commit/2d5f30e687591581586cf80b7c4ea290ece0fea8

Thanks Arthur !

Comment 15

[Arthur Chen [:arthurcc][inactive after 6/30]]

[Tracking Requested - why for this release]: