[Clock] Modules incorrectly organized

RESOLVED FIXED

Status

Firefox OS
Gaia::Clock
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jugglinmike, Assigned: jugglinmike)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

29.54 KB, patch
iliu@mozilla.com, ianliu.moz@gmail.com
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Currently, the 'alarm.js' file contains two singletons: ClockView and AlarmList. These definitions should be made in separate files that accurately reflect their contents.
(Assignee)

Comment 1

5 years ago
Created attachment 779833 [details] [diff] [review]
Improve module organization

Hey Ian,

I'm not sure how you prefer to conduct code reviews, so I've also made a pull request on GitHub; you can find it here: https://github.com/mozilla-b2g/gaia/pull/11126
Attachment #779833 - Flags: review?(iliu)
(Assignee)

Updated

5 years ago
Blocks: 897089
(In reply to Mike Pennisi [:jugglinmike] from comment #1)
> Created attachment 779833 [details] [diff] [review]
> Improve module organization
> 
> Hey Ian,
> 
> I'm not sure how you prefer to conduct code reviews, so I've also made a
> pull request on GitHub; you can find it here:
> https://github.com/mozilla-b2g/gaia/pull/11126

Hi Mike,

The reviewing process is okay for me. One bug for one request(pull request). And we can address discussion on Github if we need to do so.
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Comment on attachment 779833 [details] [diff] [review]
Improve module organization

Review of attachment 779833 [details] [diff] [review]:
-----------------------------------------------------------------

The patch works for me. It will help to organise the two modules. Thanks.
Attachment #779833 - Flags: review?(iliu) → review+
Blocks: 898666
(Assignee)

Comment 4

5 years ago
Landed on `master` with commit fb26423d9aadf53c3f9278b228791e52e8af4364

Thanks for the review, Ian!
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.