995900 - Single variant: build process for operatorvariant app

Status: VERIFIED FIXED

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

Description

[Carmen Jimenez Cabezas]

Single variant has been moved from communication/ftu to operatorvariant app.
Other customizers have been added.
We need to reflect this in the build process.

Comment 1

[Albert [:albert]]

Attachment: Patch

Comment 2

[Albert [:albert]]

In order to create the new structure of the resources folder I need to check settings in 'settings.json'. But 'settings.json' is created after building operatorvariant, the patch avoid that problem loading the settings by calling methods of 'settings.js', what doesn't seems to be a good solution.

Yuren, do you know if I can modify makefile dependencies to run 'settings.js' before calling make for each application without breaking things?

Comment 3

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

Albert, it would be okay if we don't change dependency between |settings| and |keyboard-layouts|

Comment 4

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

Albert, I found you access $PROFILE/settings.json to generate configuration files, but for the refactory plan of build system[1] which is executed by George and me, we hope all apps and configurations will be assembled in build_stage directory, so access a file from profile is that thing we don't want to happend.

however after discussed this issue with George, the better way is we generate settings.json to build_stage in settings.js and copy it to $PROFILE in |make copy-build-stage-manifest| (this rule name will be changed to "copy-stage-data" on bug 983573)

so I set this bug depend on bug 983573 and please get settings.json from build_stage in your commit after bug 983573 landed.

[1] https://wiki.mozilla.org/Gaia/Build/Refactoring_Plan

Comment 5

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

Comment on attachment 8408152 [details]
Patch

after read and test your pr, there are two major issues:
1. we have not integration test for this customization work
2. we get one (or two) more uuid json file every time we build gaia.

but according your comment on github those two issues will be resovled if you keep original name for those json files.

so set me review again if you fix those two issues, otherwise looks good to me.

and don't forget get settings.json from build_stage. I will review bug 983573's pr asap when I receive review request.

Comment 6

[Albert [:albert]]

(In reply to Yuren [:yurenju][:小朱] (PTO during 4/28~5/2) from comment #5)
> Comment on attachment 8408152 [details]
> Patch
> 
> after read and test your pr, there are two major issues:
> 1. we have not integration test for this customization work
> 2. we get one (or two) more uuid json file every time we build gaia.
> 
> but according your comment on github those two issues will be resovled if
> you keep original name for those json files.
> 
> so set me review again if you fix those two issues, otherwise looks good to
> me.
> 
> and don't forget get settings.json from build_stage. I will review bug
> 983573's pr asap when I receive review request.

As you said, issues will be solved when changing the way to determine json names, but the issue 2 is caused by a wrong clean of build_stage folder, see https://bugzilla.mozilla.org/show_bug.cgi?id=999413#c1

I'll wait to bug 983573 lands in order to change the settings origin, thanks!

Comment 7

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

Albert, review for bug 983573 cannot be done today, sorry for that. please set review flag to Alexandre Poirot (:ochameau) or  George Duan [:gduan] since I will be offline next week.

Comment 8

[Albert [:albert]]

Comment on attachment 8408152 [details]
Patch

Requested changes done

Comment 9

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

Albert, could you add some brief for each function of Resource and OperatorAppBuilder? it would help for review.

Comment 10

[Albert [:albert]]

(In reply to Yuren [:yurenju] from comment #9)
> Albert, could you add some brief for each function of Resource and
> OperatorAppBuilder? it would help for review.

Done

Comment 11

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

Comment on attachment 8408152 [details]
Patch

Albert, please see comments on github, and set me review flag again when pull request is updated, thanks!

Comment 12

[Albert [:albert]]

Comment on attachment 8408152 [details]
Patch

Changes from comment 11 (see comment in github)

Comment 13

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

Comment on attachment 8408152 [details]
Patch

looks good if lint errors are fixed! r=yurenju

Thanks Albert!

Comment 14

[Albert [:albert]]

lint errors fixed.

Master: https://github.com/mozilla-b2g/gaia/commit/6d706f80aa73cf63a83e7d86765b3236c7525dc0