1010782 - [FTU-Comms separation] FTU-Comms separation remaining tasks

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Comment 1

[Jose Manuel Cantera]

This bug tries to capture all the remaining work to have a FTU separated from comms. As it is complex we will have different parts to be reviewed under a common and coherent branch.

Comment 2

[Jose Manuel Cantera]

Attachment: import_locales.html

This part moves pending import locales to shared

Comment 3

[Jose Manuel Cantera]

Attachment: 19282.html

Part I: Move FTU To Apps directory and tweak index.html

Comment 4

[Jose Manuel Cantera]

(In reply to Jose Manuel Cantera from comment #1)
> This bug tries to capture all the remaining work to have a FTU separated
> from comms. As it is complex we will have different parts to be reviewed
> under a common and coherent branch.

That branch is 

https://github.com/jmcanterafonseca/gaia/tree/ftu_independent

Comment 5

[Fernando Campo (:fcampo)]

Comment on attachment 8423086 [details]
import_locales.html

Few comments on the commit. Please ask for r? when addressed. 
Also asking for r? to l10n peers, as this might affect their work.

Comment 6

[Jose Manuel Cantera]

Attachment: 19283.html

part that corresponds to import_init and facebook connector

Comment 7

[Jose Manuel Cantera]

Attachment: ftu_comms_iac.html

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8423137 [details]
19283.html

One comment but LGTM

Comment 9

[Jose Manuel Cantera]

Attachment: 19284.html

Comment 10

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8423138 [details]
ftu_comms_iac.html

Some comments on GH, but good job!!

Comment 11

[Fernando Campo (:fcampo)]

Comment on attachment 8423144 [details]
19284.html

Only nits on github, but everything related to languages different than english can be ignored, as those files are gonna be overwritten by the l10n team when updating translations.
And the english related nits are mostly about style and spaces, so up to you if you want to update them.

Thanks for taking care of this Jose.
Waiting for Pike's review now.

Comment 12

[Axel Hecht [:Pike]]

Sorry, but I have no idea what you're doing in this bug. I'm only seeing a bunch of string changes in the attachment I'm asked to review, and I can't make sense of it given the context.

I would have tried to look at your branch on github, but your repo doesn't seem to have master at all, so it's claiming "This branch is 8999 commits ahead and 0 commits behind master". That doesn't look inviting.

PS: if both this and the tracker had explanations of what they doing, I wouldn't need to reverse engineer this mesh of bugs

Comment 13

[Jose Manuel Cantera]

(In reply to Axel Hecht [:Pike] from comment #12)
> Sorry, but I have no idea what you're doing in this bug. I'm only seeing a
> bunch of string changes in the attachment I'm asked to review, and I can't
> make sense of it given the context.
> 
> I would have tried to look at your branch on github, but your repo doesn't
> seem to have master at all, so it's claiming "This branch is 8999 commits
> ahead and 0 commits behind master". That doesn't look inviting.
> 
> PS: if both this and the tracker had explanations of what they doing, I
> wouldn't need to reverse engineer this mesh of bugs

Axel,

In this bug we are separating the FTU as an independent app from comms. Wrt the PR you were asked to review we are only moving (import-related) literals from a contacts locales file to shared import locales files. As we are only moving literals from one file to another I believe it is no strictly necessary l10n to review the changes.

Cancelling the review request, but please let me know you are ok

thanks

Comment 14

[Axel Hecht [:Pike]]

It'd be a good idea for you to post to .l10n describing the change, some localizers might benefit from that.

Comment 15

[Fernando Campo (:fcampo)]

Comment on attachment 8423125 [details]
19282.html

Code looks nice, thanks for all the tremendous work.
Just few nits on github, optional to fulfill.

The downside is that FTU is not launching, so patch is not working.
I think it's a problem with the System part that deals with the FTU startup [system/js/ftu_launcher.js]
Now that we changed the manifest and ftu is an independent app, we should change >self._ftuURL = self._ftu.origin + self._ftu.manifest.entry_points['ftu'].launch_path;
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/ftu_launcher.js#L156

Also, Travis is still complaining with some tests.

Comment 16

[Jose Manuel Cantera]

landed remaining import part:

https://github.com/jmcanterafonseca/gaia/commit/7c3285eaff973afa4a84edfb994e0b9d58ffb54a

Comment 17

[Jose Manuel Cantera]

landed locales part:

https://github.com/mozilla-b2g/gaia/commit/9078b767042551e5d21637c83d40f1aa02041cf6

Comment 18

[Jose Manuel Cantera]

landed Contacts IAC part:

https://github.com/mozilla-b2g/gaia/commit/2c45cebc622141d7e2726dc9fb09eeaa388ac7b2

Comment 19

[Jose Manuel Cantera]

Attachment: 19383.html

Comment 20

[Jose Manuel Cantera]

This is the final PR that will make the FTU an app separated from Comms. Please

:Yuren, Could you review the ftu/build part?
:Evelyn, Could you review the settings part?
:Fcampo, Could you review the FTU part?
:Alive, Could you review the ftu launcher part?

thanks, all

Comment 21

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8424836 [details]
19383.html

only change ftu manifest url to ftu instead of communications.

also set feedback to Pike since you added a new app and maybe we need a new directory in l10n repository.

Comment 22

[Francesco Lodolo [:flod]]

Comment on attachment 8424836 [details]
19383.html

That's one big diff.

From a l10n point of view there's nothing to check: .properties files are moved from /communications/ftu to /ftu, no manual creation of folders involved (consider that master has already 6 more folders than 1.4).

I'll send a message out to dev-l10n when this land to warn that they can copy the old file in the new path: that will help people working directly on Hg, probably not much people working on external tools.

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8424836 [details]
19383.html

r+ for system app part. Thank you for doing this.

Comment 24

[Axel Hecht [:Pike]]

Yes, folders are created by the automation I use for the conversion, notably, files matching https://bitbucket.org/pike/gaiaconv/src/default/gaiaconv/filemap.py?at=default#cl-35 get picked up.

Comment 25

[Evelyn Hung]

Comment on attachment 8424836 [details]
19383.html

Trivial modification, r+. Thank you! :)

Comment 26

[Fernando Campo (:fcampo)]

Comment on attachment 8424836 [details]
19383.html

After the last changes on locales and callscreen working again, tested on device and looking great :D

Superb job!

Comment 27

[Jose Manuel Cantera]

final commit: 

https://github.com/mozilla-b2g/gaia/commit/511bd17cf3fe9197676a71f04f63b54a29dbe51a