Closed Bug 990002 Opened 7 years ago Closed 7 years ago

[Import] Move Utilities to shared

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S5 (11apr)
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: fcampo, Assigned: fcampo)

References

Details

Attachments

(1 file)

As part of the [IMPORT] externalizing process, we need to move a few of the utilities from Contacts, that are used on FTE, to a shared folder.

This utilities are mostly related with the import process, or with visual styles that need to be the same (overlay, status).
Depends on: 990079
Depends on: 990081
Assignee: nobody → fernando.campo
Asking for 2 reviews for the different parts (Contacts, FTU)
Attachment #8400576 - Flags: review?(jmcf)
Attachment #8400576 - Flags: review?(francisco.jordano)
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

Thanks for taking this, I'm r- basically for:

- The renaming, IMHO, should be something like |shared/js/contacts/import| ... just having a |import| folder directly in shared javascript folder could be ambiguous.
- Test failing in travis (both ui tests and marionette) are specifically related to contacts (form and activities), so we should double check them.

Thanks Fernando.
Attachment #8400576 - Flags: review?(francisco.jordano) → review-
(In reply to Francisco Jordano [:arcturus] from comment #2)
> Comment on attachment 8400576 [details] [review]
> Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896
> 
> Thanks for taking this, I'm r- basically for:
> 
> - The renaming, IMHO, should be something like |shared/js/contacts/import|
> ... just having a |import| folder directly in shared javascript folder could
> be ambiguous.

Yes, I agree

> - Test failing in travis (both ui tests and marionette) are specifically
> related to contacts (form and activities), so we should double check them.
> 
> Thanks Fernando.
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

cancelling my review. Left comments on GH.
Attachment #8400576 - Flags: review?(jmcf)
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

Comments addressed on the updated PR.

Travis is quite playful today :S
Attachment #8400576 - Flags: review?(jmcf)
Attachment #8400576 - Flags: review?(francisco.jordano)
Attachment #8400576 - Flags: review-
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

From the FTU, only a small comment to address. R+ when addressed.
Attachment #8400576 - Flags: review+
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

As Francisco is away for some days (thanks for the quick review Borja!), I'm adding Etienne for the Dialer part (just changes of paths)
Attachment #8400576 - Flags: review?(francisco.jordano) → review?(etienne)
No longer depends on: 990079, 990081
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

sure, r=me for the dialer part (tiny nit on github)
Attachment #8400576 - Flags: review?(etienne) → review+
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

r+ for the contacts part conditioned to:

a/ address recent comments on GH
b/ Green Travis
c/ a new round of smoke tests performed over FTU, Contacts Import, and Dialer parts. 

Please do not land until a, b, and c are met. 

Thanks, Fernando, good work!
Attachment #8400576 - Flags: review?(jmcf) → review+
a, b, c met (hopefully no regressions!)

merged on master - 1acd82e474c2df2bb3af3a33f725c34a109764db

thanks all for review and tips.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S5 (11apr)
You need to log in before you can comment on or make changes to this bug.