990002 - [Import] Move Utilities to shared

Status: RESOLVED FIXED

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

Description

[Fernando Campo (:fcampo)]

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).

Comment 1

[Fernando Campo (:fcampo)]

Attachment: Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

Asking for 2 reviews for the different parts (Contacts, FTU)

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

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.

Comment 3

[Jose Manuel Cantera]

(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 4

[Jose Manuel Cantera]

Comment on attachment 8400576 [details] [review]
Link to PR - https://github.com/mozilla-b2g/gaia/pull/17896

cancelling my review. Left comments on GH.

Comment 5

[Fernando Campo (:fcampo)]

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

Comment 6

[Borja Salguero [:borjasalguero]]

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.

Comment 7

[Fernando Campo (:fcampo)]

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)

Comment 8

[Etienne Segonzac (:etienne)]

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)

Comment 9

[Jose Manuel Cantera]

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!

Comment 10

[Fernando Campo (:fcampo)]

a, b, c met (hopefully no regressions!)

merged on master - 1acd82e474c2df2bb3af3a33f725c34a109764db

thanks all for review and tips.