Closed Bug 1239828 Opened 8 years ago Closed 8 years ago

Enable Loop specific L10n in mozilla-central

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox45 fixed, firefox46 fixed, firefox47 fixed)

RESOLVED FIXED
mozilla47
Iteration:
47.1 - Feb 8
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(9 files, 8 obsolete files)

40 bytes, text/x-github-pull-request
Mardak
: review+
Details | Review
12.94 KB, patch
Mardak
: review+
Details | Diff | Splinter Review
3.95 MB, patch
standard8
: review+
Details | Diff | Splinter Review
6.45 KB, patch
glandium
: review+
Details | Diff | Splinter Review
40 bytes, text/x-github-pull-request
Mardak
: review+
Details | Review
17.03 KB, patch
Details | Diff | Splinter Review
4.10 MB, patch
Details | Diff | Splinter Review
6.50 KB, patch
Details | Diff | Splinter Review
17.12 KB, patch
Details | Diff | Splinter Review
When I tried importing Loop into mozilla-central, the try server builds failed due to an issue with repacks and trying to repack loop.

As we're using a different process for L!0n at the moment, we're going to need to find a way to avoid the repacks happen, so we just package our own items.
Rank: 8
Attachment #8709965 - Attachment is obsolete: true
Attached patch Update Loop to latest version. (obsolete) — Splinter Review
Export from github repo to m-c which includes the locale updates.
Axel, Mike, I'm looking for some suggestions here.

What we had agreed to do for Loop (for now at least), was to package up all the locales into its own xpi, and ship as part of the xpi. We've done that in our git repo and it works fine. For the Firefox builds that ship, I was assuming we'd do the same, as otherwise we'd be fiddling multiple l10n repos and sign-offs, which wouldn't be good.

However, m-c is giving me some issues - it doesn't like the fact that there's multiple locales listed in the jar.mn file, and hence the repacks fail (Multiple locales aren't supported).

This is the patch that I've tried so far, but I don't understand enough about the finders to know if this is something we can easily whitelist to avoid repacks altogether.

Any ideas?
Attachment #8709968 - Flags: feedback?(mh+mozilla)
Attachment #8709968 - Flags: feedback?(l10n)
That code is out of my ballpark, which is also why I didn't think about that as a potential cause of trouble in our plan. I think the needinfo on glandium is right here.

My personal assumption is that we might end up with more code that's coming with l10n for a subset of locales, as part of go-faster etc.
(In reply to Mark Banner (:standard8) from comment #3)
> Created attachment 8709968 [details] [diff] [review]
> Make Loop's locale system work with the l10n repacks.
> 
> Axel, Mike, I'm looking for some suggestions here.
> 
> What we had agreed to do for Loop (for now at least), was to package up all
> the locales into its own xpi, and ship as part of the xpi. We've done that
> in our git repo and it works fine.

I understand why you'd want to do that but...

> For the Firefox builds that ship, I was
> assuming we'd do the same, as otherwise we'd be fiddling multiple l10n repos
> and sign-offs, which wouldn't be good.

I don't understand why we'd want to ship all locales everywhere. Loop is no different from all the other things in Firefox in that respect.
(In reply to Mike Hommey [:glandium] from comment #5)
> (In reply to Mark Banner (:standard8) from comment #3)
> > For the Firefox builds that ship, I was
> > assuming we'd do the same, as otherwise we'd be fiddling multiple l10n repos
> > and sign-offs, which wouldn't be good.
> 
> I don't understand why we'd want to ship all locales everywhere. Loop is no
> different from all the other things in Firefox in that respect.

My main thought was to keep the system add-on that gets shipped as an xpi update, the same as the system add-on that gets shipped in Firefox. Hence as a side-effect keeping all locales in FF.

I know that isn't as idea from a download/disk space perspective though.

So maybe we can find a way to include just one locale plus en-US (due to how our fallback works)?

However, with the constraint that we can't put the files in the l10n repositories (because that isn't where they are being localised now), I'm not sure I can see a different way around this at the moment. We can't ifdef jar.mn per locale I believe, as the repack wouldn't work.

I'll continue thinking this afternoon, but I'm struggling here with finding good options.
(In reply to Mark Banner (:standard8) from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #5)
> > (In reply to Mark Banner (:standard8) from comment #3)
> > > For the Firefox builds that ship, I was
> > > assuming we'd do the same, as otherwise we'd be fiddling multiple l10n repos
> > > and sign-offs, which wouldn't be good.
> > 
> > I don't understand why we'd want to ship all locales everywhere. Loop is no
> > different from all the other things in Firefox in that respect.
> 
> My main thought was to keep the system add-on that gets shipped as an xpi
> update, the same as the system add-on that gets shipped in Firefox. Hence as
> a side-effect keeping all locales in FF.
> 
> I know that isn't as idea from a download/disk space perspective though.
> 
> So maybe we can find a way to include just one locale plus en-US (due to how
> our fallback works)?

Firefox localized builds don't include en-US. They include missing strings as en-US by merging the locales first.

> However, with the constraint that we can't put the files in the l10n
> repositories (because that isn't where they are being localised now)

"It isn't where they are being localised now" is not really specifying why you "can't" put the files in the l10n repositories. I can see it as cumbersome, but certainly not as a definite blocker.

>, I'm
> not sure I can see a different way around this at the moment. We can't ifdef
> jar.mn per locale I believe, as the repack wouldn't work.

Actually, I think you can ifdef per locale.
(In reply to Mike Hommey [:glandium] from comment #7)
> (In reply to Mark Banner (:standard8) from comment #6)
> > So maybe we can find a way to include just one locale plus en-US (due to how
> > our fallback works)?
> 
> Firefox localized builds don't include en-US. They include missing strings
> as en-US by merging the locales first.

Our code currently works in a different way, as it is based around web-dev processes. The overhead to have both isn't huge here.

> > However, with the constraint that we can't put the files in the l10n
> > repositories (because that isn't where they are being localised now)
> 
> "It isn't where they are being localised now" is not really specifying why
> you "can't" put the files in the l10n repositories. I can see it as
> cumbersome, but certainly not as a definite blocker.

Actually it is. Not only would we have to put the files into all the l10n repositories, we'd also have to request sign-offs for all of them, update the l10n dashboard - some of the l10n sign-offs might also be blocked due to other reasons (e.g. bad changes on the tree).

We'd also need to be able to do this in the release channel, as we need to be able to land there in case of a security release. There's no sign-off dashboards for the release channel, so without writing those or some work around, we wouldn't be able to update the files.

> >, I'm
> > not sure I can see a different way around this at the moment. We can't ifdef
> > jar.mn per locale I believe, as the repack wouldn't work.
> 
> Actually, I think you can ifdef per locale.

Hmm, I guess this might work, I'll give it a try.
Comment on attachment 8709968 [details] [diff] [review]
Make Loop's locale system work with the l10n repacks.

I've been doing various experimenting, and here's where I currently am:

- Using ifdefs don't work - the jar.mn isn't processed for the repacks, only the output chrome.manifest is.
- Importing the loop.properties to the locale repos, isn't really feasible at this time. The main reason being that there is no mechanism in place for updating sign-offs on the release channel. This we would be a requirement as for go faster, we'll need to ensure the latest version of the add-on is always landed in the release channel - in case of a security chemspill (otherwise users might get downgraded for a day or two).

Unfortunately, we're now about a week or so away from needing a solution, so that we can move forward with go faster in 45.

The current possibilities are:

a) Do some extra L10n repack hacks to get loop.properties from the source tree rather than l10n trees.
b) Include all the locales in the build as "content" files, rather than locale files.

We think the b) route would increase the size of the packages by about 90k.
Attachment #8709968 - Flags: feedback?(mh+mozilla)
Attachment #8709968 - Flags: feedback?(l10n)
I don't have a strong opinion here myself.
(In reply to Mark Banner (:standard8) from comment #9)
> Comment on attachment 8709968 [details] [diff] [review]
> Make Loop's locale system work with the l10n repacks.
> 
> I've been doing various experimenting, and here's where I currently am:
> 
> - Using ifdefs don't work - the jar.mn isn't processed for the repacks, only
> the output chrome.manifest is.

Then you have another problem, because jar.mn *are* processed during repacks. If yours is not, it means your directory is not recursed during a repack, which means you need to adjust browser/locales/Makefile.in.
This imports Loop at the latest version (changeset 0fd9adb70a8976bd6abd071844236bb96f9aeee2) to make the L10n patch hooking up easier.
Attachment #8714854 - Flags: review+
Attachment #8709967 - Attachment is obsolete: true
Attachment #8709968 - Attachment is obsolete: true
Mike, this implements the locale hooks as discussed. I've tested it on try and it works, and doing manual repacks locally seem to work as well.
Attachment #8714859 - Flags: review?(mh+mozilla)
Comment on attachment 8714860 [details] [review]
[merged][loop] Standard8:bug-1239828-locale > mozilla:master

These are the changes required in the loop repo.

There's a couple of script changes. I changed the chrome.manifest generation to a script as that was better for the slightly more complex changes required.

I've also removed the previous workarounds in the Makefile that were stripping out the l10n stuff.
Attachment #8714860 - Flags: review?(edilee)
This tidies up the remaining strings in loop.properties. There's one string still used (by webrtcui.jsm), I'll file a follow-up bug to clean that up completely.
Attachment #8714872 - Flags: review?(edilee)
Comment on attachment 8714860 [details] [review]
[merged][loop] Standard8:bug-1239828-locale > mozilla:master

r=Mardak with various comments addressed.

Keeping track of the generated chrome.manifest from a preprocessed jar.mn on top of registration syntax of each file hurts my head. ;)
Attachment #8714860 - Flags: review?(edilee) → review+
Comment on attachment 8714872 [details] [diff] [review]
Now that Loop's system-addon L10n is hooked up, remove redundant strings.

flod, is there any particular timing we should consider when removing these strings? Are localizers using them to translate the ones in loop-client-l10n?

Standard8, I suppose there's a window of potential brokenness of old add-on on new nightly, but most people will only have it via system add-on which will be updated at the same time/before the strings are removed.
Flags: needinfo?(francesco.lodolo)
(In reply to Ed Lee :Mardak from comment #18)
> flod, is there any particular timing we should consider when removing these
> strings? Are localizers using them to translate the ones in loop-client-l10n?

If you're going to remove them from mozilla-central and let the change ride the trains, I don't see particular issues with timing, they still have access to the file in mozilla-aurora if they need.
Flags: needinfo?(francesco.lodolo)
Corrected patch - missed a couple of removes before.
Attachment #8714985 - Flags: review+
Attachment #8714854 - Attachment is obsolete: true
Attachment #8714859 - Attachment is obsolete: true
Attachment #8714859 - Flags: review?(mh+mozilla)
Updated fallback options per Ed's review comments on the loop repo PR.
Attachment #8714987 - Flags: review?(mh+mozilla)
Comment on attachment 8714987 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Review of attachment 8714987 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/extensions/loop/jar.mn
@@ +45,5 @@
>  % override chrome://loop/skin/menuPanel@2x.png    chrome://loop/skin/menuPanel-aero@2x.png        os=WINNT osversion=6.1
>  % override chrome://loop/skin/toolbar.png         chrome://loop/skin/toolbar-XP.png               os=WINNT osversion<6
>  % override chrome://loop/skin/toolbar.png         chrome://loop/skin/toolbar-aero.png             os=WINNT osversion=6
>  % override chrome://loop/skin/toolbar.png         chrome://loop/skin/toolbar-aero.png             os=WINNT osversion=6.1
> +% override chrome://loop/skin/toolbar.png         chrome://loop/skin/toolbar-win8.png            os=WINNT osversion=6.2

Please leave those non-l10n changes for a separate patch.

@@ +53,5 @@
>  % override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-aero@2x.png          os=WINNT osversion=6.1
> +% override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-win8@2x.png         os=WINNT osversion=6.2
> +% override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-win8@2x.png         os=WINNT osversion=6.3
> +  # Due to the way Loop's L10n works, we include a fallback to en-US.
> +  content/locale-fallback/en-US/          (chrome/locale/en-US/*)

You don't need the fallback on the builds that use the en-US files (en-US locale included).

::: browser/extensions/loop/moz.build
@@ +15,5 @@
>  JAR_MANIFESTS += ['jar.mn']
>  
>  XPCSHELL_TESTS_MANIFESTS += ['chrome/test/xpcshell/xpcshell.ini']
>  
> +DEFINES['AB_CD'] = CONFIG['MOZ_UI_LOCALE']

This shouldn't be needed.
Attachment #8714987 - Flags: review?(mh+mozilla) → feedback+
Moved the non-locale jar.mn changes into this patch.
Attachment #8715257 - Flags: review+
Mike, this is the updated patch but without the fallback removals - which I haven't been able to get to work. I'll attach a patch/explanation in a moment.

If there's not an easy fix for the fallback items, would you be happy if we landed this and split that out to a separate bug?
Attachment #8715258 - Flags: review?(mh+mozilla)
Attachment #8714985 - Attachment is obsolete: true
Attachment #8714987 - Attachment is obsolete: true
This was my attempt for removing the fallback from the en-US locale. For testing this I was:

- Setting up a mozconfig with l10n base & disable compile environment specified.
- ./mach configure
- Copy the packaged en-US build
- cd obj-l10n/browser/locales
- make merge-fr LOCALE_MERGEDIR=`pwd`/mergedir
- make installers-fr LOCALE_MERGEDIR=`pwd`/mergedir

In the packaged build on the output, the chrome/content/locale-fallback directory isn't there, and the content line isn't in the manifest.

If I looked into obj-l10n/dist/xpi-stage/locale/fr/browser/features/loop@mozilla.org/ then both of those were present.

This leads me to think that the make/jar handling was working fine, but something about the merge isn't working correctly in this case.
Comment on attachment 8714872 [details] [diff] [review]
Now that Loop's system-addon L10n is hooked up, remove redundant strings.

(In reply to Mark Banner (:standard8) from comment #16)
> This tidies up the remaining strings in loop.properties. There's one string
> still used (by webrtcui.jsm), I'll file a follow-up bug to clean that up
> completely.

> ## LOCALIZATION NOTE(clientShortname2): This should not be localized and
> ## should remain "Firefox Hello" for all locales.
> clientShortname2=Firefox Hello
Might as well just delete the whole file and inline `host = "Firefox Hello";`
https://dxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI.jsm#261

And remove the jar.mn entry:
https://dxr.mozilla.org/mozilla-central/source/browser/locales/jar.mn#30
Attachment #8714872 - Flags: review?(edilee) → review+
Comment on attachment 8715258 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Review of attachment 8715258 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/extensions/loop/jar.mn
@@ +53,5 @@
>  % override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-aero@2x.png          os=WINNT osversion=6.1
>  % override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-win8@2x.png         os=WINNT osversion=6.2
>  % override chrome://loop/skin/toolbar@2x.png      chrome://loop/skin/toolbar-win8@2x.png         os=WINNT osversion=6.3
> +  # Due to the way Loop's L10n works, we include a fallback to en-US.
> +  content/locale-fallback/en-US/          (chrome/locale/en-US/*)

It seems to me you should be able to move this line and the corresponding % content in the large #if, because the fallback is only really needed on the locales that are *not* using a copy of the en-US locale.
Then you can just put the loadAllStrings call in a try{}catch. I don't see why that wouldn't work.

That being said, it should even be possible to not have a fallback *at* *all*, by actually merging the locales like we do for everything else. But I'm willing to keep that to a followup.
Attachment #8715258 - Flags: review?(mh+mozilla) → feedback+
Comment on attachment 8715258 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Review of attachment 8715258 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, rereading your comment, I see what's going on, l10n-repack is not adding new content, it only replaces locale stuff. So it ignores the fallback entirely. The right thing to do, then, is to merge the locales and get rid of the fallback entirely, in a followup.
Attachment #8715258 - Flags: feedback+ → review+
(In reply to Mike Hommey [:glandium] from comment #28)
The right thing to do, then, is to merge the locales and get rid
> of the fallback entirely, in a followup.

Filed bug 1245785.

(In reply to Ed Lee :Mardak from comment #26)
> > ## LOCALIZATION NOTE(clientShortname2): This should not be localized and
> > ## should remain "Firefox Hello" for all locales.
> > clientShortname2=Firefox Hello
> Might as well just delete the whole file and inline `host = "Firefox Hello";`
> https://dxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI.
> jsm#261
> 
> And remove the jar.mn entry:
> https://dxr.mozilla.org/mozilla-central/source/browser/locales/jar.mn#30

Yeah, I considered that, but I think we can do better anyway - now we're an add-on I'd like to see if we can move the code to the add-on and have some sort of hook to allow us to set this. Which would be especially useful if we ever rebranded. I've raised bug 1245786 to cover this and do the work as a whole.
Blocks: 1245786, 1245785
Attachment #8715261 - Attachment is obsolete: true
Comment on attachment 8714860 [details] [review]
[merged][loop] Standard8:bug-1239828-locale > mozilla:master

https://github.com/mozilla/loop/commit/959bde3a3ecf43b0b7327701367d309238147bc6
https://github.com/mozilla/loop/commit/7f3734171fcef433116391e2fc5e1be6f27d742c
Attachment #8714860 - Attachment description: [loop] Standard8:bug-1239828-locale > mozilla:master → [merged][loop] Standard8:bug-1239828-locale > mozilla:master
https://hg.mozilla.org/integration/fx-team/rev/b18fabcd0c07e43c575d6cd302d040105098a7cc
Bug 1239828 - Update Loop to the latest version. r=Standard8 for import of already reviewed code.

https://hg.mozilla.org/integration/fx-team/rev/7fc621cd58bff3547cf7d4ed427782d6397400f1
Bug 1239828 - Support Loop's localisation mechanisms for Loop as a system-addon. r=glandium

https://hg.mozilla.org/integration/fx-team/rev/d577f0c34dee0bf5d0054bea3d8535af03f2b85d
Bug 1239828 - Now that Loop's system-addon L10n is hooked up, remove redundant strings. r=Mardak
Iteration: --- → 47.1 - Feb 8
Blocks: 1245912
Comment on attachment 8715257 [details] [diff] [review]
Update Loop to the latest version. r=Standard8 for import of already reviewed code.

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello system-addon.
[User impact if declined]: This is a partial update to the system add-on, with the primary aim of importing the L10n files from Hello's special system and removing the dependencies on Firefox's internal strings, hence allowing feature changes at any time - part of the go faster plan.
[Describe test coverage new/current, TreeHerder]: Landed in m-c. Add-on has tests. Also being tested in a separate repo.
[Risks and why]: Infrastructure update for the add-on to prepare it for updates during general release
[String/UUID change made/needed]: None

We'll need to get this uplifted to 45 as well. I'm working on fixing bitrot for those patches.
Attachment #8715257 - Flags: approval-mozilla-aurora?
Comment on attachment 8715258 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Approval Request Comment:

Please see comment 33
Attachment #8715258 - Flags: approval-mozilla-aurora?
Attachment #8716334 - Flags: review?(edilee)
Attachment #8716334 - Flags: review?(dmose)
Attachment #8716334 - Flags: review?(edilee)
Attachment #8716334 - Flags: review?(dmose)
Attachment #8716334 - Flags: review+
Comment on attachment 8716334 [details] [review]
[merged][loop] Standard8:bug-1239828-l10n > mozilla:master

https://github.com/mozilla/loop/commit/379b794aee6de61f2ea5789916f29fe81b3b3e73
Attachment #8716334 - Attachment description: [loop] Standard8:bug-1239828-l10n > mozilla:master → [merged][loop] Standard8:bug-1239828-l10n > mozilla:master
This updates the ifdefs with the correct values from the result of the PR. We also add the 'id' locale that was being ignored by .gitignore (.hgignore is slightly better, so I've added an exclusion to .gitignore only).
Attachment #8716360 - Flags: review?(edilee)
Comment on attachment 8716360 [details] [diff] [review]
Fix broken L10n repos, correct ifdef for inclusion of locales, and add 'id' locale that was being ignored by .gitignore.

I got rs=mikedeboer over irc
Attachment #8716360 - Flags: review?(edilee)
https://hg.mozilla.org/integration/fx-team/rev/e5daa0514aea68fedd729cfe44dc02c3c2042e3b
Bug 1239828 - Fix broken L10n repos, correct ifdef for inclusion of locales, and add 'id' locale that was being ignored by .gitignore. rs=mikedeboer
Comment on attachment 8716360 [details] [diff] [review]
Fix broken L10n repos, correct ifdef for inclusion of locales, and add 'id' locale that was being ignored by .gitignore.

Approval Request Comment

This fixes issues with L10n repacks. We'll need this patch as well.
Attachment #8716360 - Flags: approval-mozilla-aurora?
... and windows is busted for all locales :-/, seems a path '/' vs '\' and/or escaping thereof problem.
Reopening due to the Windows L10n issue. I'm building this on my VM, which is likely to take me into tomorrow.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/e5daa0514aea
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Bah, thought I'd put leave open on it.
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
Depends on: 1246511
Comment on attachment 8715257 [details] [diff] [review]
Update Loop to the latest version. r=Standard8 for import of already reviewed code.

Cancelling approval requests until we can find a fix for l10n repacks.
Attachment #8715257 - Flags: approval-mozilla-aurora?
Attachment #8715258 - Flags: approval-mozilla-aurora?
Attachment #8716360 - Flags: approval-mozilla-aurora?
Depends on: 1246592
(In reply to Axel Hecht [:Pike] from comment #41)
> ... and windows is busted for all locales :-/, seems a path '/' vs '\'
> and/or escaping thereof problem.

I've split this out to bug 1246592 - only to make it clearer as to what's happening, its still top priority to fix.
Comment on attachment 8715257 [details] [diff] [review]
Update Loop to the latest version. r=Standard8 for import of already reviewed code.

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello system-addon.
[User impact if declined]: This is a partial update to the system add-on, with the primary aim of importing the L10n files from Hello's special system and removing the dependencies on Firefox's internal strings, hence allowing feature changes at any time - part of the go faster plan.
[Describe test coverage new/current, TreeHerder]: Landed in m-c. Add-on has tests. Also being tested in a separate repo.
[Risks and why]: Infrastructure update for the add-on to prepare it for updates during general release
[String/UUID change made/needed]: None

This will also need the fix for bug 1246592.

We'll need to get this uplifted to 45 as well. I'm working on fixing bitrot for those patches.
Attachment #8715257 - Flags: approval-mozilla-aurora?
Bug 1246592 is now fixed, so re-closing this.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Keywords: leave-open
Resolution: --- → FIXED
Comment on attachment 8715258 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Approval Request Comment

Please see comment 47.
Attachment #8715258 - Flags: approval-mozilla-aurora?
Comment on attachment 8716360 [details] [diff] [review]
Fix broken L10n repos, correct ifdef for inclusion of locales, and add 'id' locale that was being ignored by .gitignore.

Approval Request Comment

Please see comment 47.
Attachment #8716360 - Flags: approval-mozilla-aurora?
Approval Request Comment

Beta version of the first patch, please see comment 47 for details.
Attachment #8717541 - Flags: approval-mozilla-beta?
Approval Request Comment

Beta version of the second patch, please see comment 47 for details.
Attachment #8717543 - Flags: approval-mozilla-beta?
Attachment #8717543 - Attachment is patch: true
Approval Request Comment

Beta version of the third patch, please see comment 47 for details.
Attachment #8717545 - Flags: approval-mozilla-beta?
Blocks: 1247424
Comment on attachment 8715258 [details] [diff] [review]
Support Loop's localisation mechanisms for Loop as a system-addon.

Part of a large uplift to switch Hello locale managment to system addons.
OK to uplift to aurora.
Attachment #8715258 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8715257 [details] [diff] [review]
Update Loop to the latest version. r=Standard8 for import of already reviewed code.

Switching locales for Hello to system addons. OK to uplift to aurora.
Attachment #8715257 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8716360 [details] [diff] [review]
Fix broken L10n repos, correct ifdef for inclusion of locales, and add 'id' locale that was being ignored by .gitignore.

All the Hello locale things...  In Aurora.
Attachment #8716360 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8717543 [details] [diff] [review]
[beta patch] Support Loop's localisation mechanisms for Loop as a system-addon.

Should be in 45 beta 6.
Attachment #8717543 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8717545 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8717541 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
seems its still busted on beta
Flags: needinfo?(standard8)
I know I was pushing the limits a lot, but it would have been nice to have been pinged over irc before a backout happened - especially as I was literally just about to push a patch I had tested locally. As it was, I've had to fight with hg to get patches re-applied (graft doesn't work for me for binary, and transplant just doesn't after backouts).

Given I'm meant to be on PTO today, I've pushed the fix to try to double check as it doesn't matter now. I'll see if I get time later to re-push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=15b156e81edb
Flags: needinfo?(standard8)
> I know I was pushing the limits a lot, but it would have been nice to have
> been pinged over irc before a backout happened
Sorry about that: we didn't have news from you, the next go to build in Thursday, I made the call to have a clean green tree to make progress on the rest of the issues.
Yeah, I'll comment in the bug the next time.

Try run is green - I had to uplift bug 1228998 to fix the bustage. Since that's basically just a build-config only + bustage fix that's well baked on central+aurora, I assumed that was ok.
backed out by request from sylvestre in https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=8bf2c5452d44
Flags: needinfo?(standard8)
Flags: needinfo?(standard8)
You need to log in before you can comment on or make changes to this bug.