Closed Bug 1055043 Opened 5 years ago Closed 5 years ago

[Music] modularize the views

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.2+, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S8 (7Nov)
feature-b2g 2.2+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.2 --- fixed

People

(Reporter: dkuo, Assigned: dkuo)

References

Details

Attachments

(2 files)

wip
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
squib
: review+
Details | Review
Besides the player's logic, currently the music app put all the views logic together in music.js, for the first step on refactoring, we are splitting all these views into different modules and files so that music app is better manageable, and will be ready to go to the next refactoring step.
Attached file wip
Attached file patch
Jim,

This bug is phase 1 for music refactoring(Bug 1053677), and here comes the first patch, basically I did:

1. Split music.js, put them into new modules/files under ui and views folders.
2. Add App object to represent Music app itself.
3. Fix the music jshint errors(except metadata.js).

It's a huge patch but most of them are the original code from music.js, would you please review this? thanks!!!
Attachment #8500334 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8500334 [details] [review]
patch

This looks pretty good. I have some comments on GitHub about various small issues.

You'll also need to update the patch, since it's bitrotted (there are a couple of changes from bug 841949 and some more from bug 841977). Thanks!
Attachment #8500334 - Flags: review?(squibblyflabbetydoo) → review-
(In reply to Jim Porter (:squib) from comment #3)
> Comment on attachment 8500334 [details] [review]
> patch
> 
> This looks pretty good. I have some comments on GitHub about various small
> issues.
> 
> You'll also need to update the patch, since it's bitrotted (there are a
> couple of changes from bug 841949 and some more from bug 841977). Thanks!

Thanks Jim!!! after I have addressed those issues and will rebase to the latest, then ask review from you again.
Comment on attachment 8500334 [details] [review]
patch

Jim, I have updated my patch and basically I did:

1. Addressed the issues you mentioned on github.
2. Rebased to the master head.
3. Resolved the conflicts.

Would you please review this again? thanks!
Attachment #8500334 - Flags: review- → review?(squibblyflabbetydoo)
Attachment #8500334 - Flags: review?(squibblyflabbetydoo) → review+
Blocks: 838035
Thanks Jim!

master: https://github.com/mozilla-b2g/gaia/commit/f3bb9e92ef540aeeb464ecb801dd867c42072cda
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.2+
Blocks: 2.2-music
Target Milestone: --- → 2.1 S8 (7Nov)
Blocks: 1103254
You need to log in before you can comment on or make changes to this bug.