1039219 - Migrate all snippets for preferences in Makefile to preferences.js

Status: RESOLVED FIXED

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

Description

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

it is not necessary to handle preferences stuffs both in Makefile and preferences.js, we can migrate it into preferences.js.

Comment 1

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

Attachment: github PR: https://github.com/mozilla-b2g/gaia/pull/21809

George, could you review this pr?

I have verified:
1. DOGFOOD=1 make preferences
2. PRODUCTION=1 make preferences
3. make preferences

all result is same between master and my branch, additional we have build.test.js to check preference between engineer and produciton mode.

and we also should file another bug to move preference rule to pre-app or post-app.

Comment 2

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

https://tbpl.mozilla.org/?rev=705b4bfc7f5b58843b0ea1b72b221495f80b8d65&tree=Gaia-Try

Comment 3

[Leila]

Nice Yuren, how about changing the override and overridePreferences function names to "add" or "append" and "addPreference" or "appendPreference"? For bug 1037395, I'm thinking of adding a flag, something like override=true/false to this function.

Comment 4

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

that makes sense.

Comment 5

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

but we should always orverride preference if we have the same one come from EXTENDED_PREF_FILES or PARTNER_PREF_FILES.

for bug 1037395, we can execute extended and partner javascript and create a |user_pref()| in sandbox to store pref in a object, then we can override preference without string processing or regex.

like this: https://github.com/mozilla-b2g/gaia/blob/master/build/test/integration/helper.js#L6-L20

that is for nodejs, but we need to have a xpcshell environment version.

Comment 6

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

so I will keep using override/overridePreference

Comment 7

[Leila]

According to Makefile EXTENDED_PREF_FILES are optional files that may be provided to _extend_ the set of default preferences installed for gaia while PARTNER_PREF_FILES are optional partner provided preference files while will be added after the ones on the EXTENDED_PREF_FILES. I'm not sure if the comments are relevant, but if you think they should be treated the same and have a check for both to override, then we will do so.

Comment 8

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8456771 [details] [review]
github PR: https://github.com/mozilla-b2g/gaia/pull/21809

Tbpl fails due to getFile(GAIA_DISTRIBUTION_DIR).
Please fix it.

For the rest parts looks good looks good. It keeps the same logic as original makefile.

Comment 9

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

Leila, you are right, I will change |override| to |append|, thanks your observation.

Comment 10

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

https://tbpl.mozilla.org/?rev=b19ac369abb724e187facab05c73828e3946a0e6&tree=Gaia-Try

Comment 11

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

Comment on attachment 8456771 [details] [review]
github PR: https://github.com/mozilla-b2g/gaia/pull/21809

looks good.

George, can you review this pr? I also added unit test for it.

Comment 12

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8456771 [details] [review]
github PR: https://github.com/mozilla-b2g/gaia/pull/21809

Patch looks fine. I think we can put this task into pre-app.js.

Comment 13

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

bug 1050709 is filed for move it into pre-app.js.

Comment 14

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

rebase, squash and add more unit test:

https://tbpl.mozilla.org/?rev=18bc91795b0e32c6676c655c6d3a2788da283e0b&tree=Gaia-Try

Comment 15

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

merged.

https://github.com/mozilla-b2g/gaia/commit/833fad063e3a282f0d0f3af0e022a58e68159f69