Move exceptions from compare-locales to $(app)/locales

RESOLVED FIXED

Status

Mozilla Localizations
Infrastructure
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: Pike, Assigned: Pike)

Tracking

({verified1.8.1.8})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Currently, all implementations of compare-locales have hard-coded logic to ignore some entities and files in the code itself.

At least for the python version, this should be moved out. I'm using plain python functions just because it's easier to do, and allows for significant flexibility when hacking those functions.

The fix is twofold, one is to actually make the python code look for the python files, load them and do the right thing, and the other is to land the exception code on trunk and branch.

The exception code is pretty simple, with some reordering and simplification compared to what used to be in CompareLocales.py. I just dropped regular expressions where I don't need them.

In CompareLocales.py, I moved the filtering logic into the function that actually knows about the module structure, it made a better fit there. I need to pass the fltr function around a bit to pick it up both for the file collection part as well as for the entity comparison part.

Rob, do you have time to review this?
Created attachment 280077 [details] [diff] [review]
call out to $(app)/locales/filter.py for exceptions
Assignee: nobody → l10n
Status: NEW → ASSIGNED
Attachment #280077 - Flags: review?(rcampbell)
Created attachment 280079 [details] [diff] [review]
exceptions for Firefox and Thunderbird

The actual filters are probably something for Benjamin to review, both for file name and location and for the stuff to ignore. This should correspond to what we used to ignore in compare-locales before, either explicitly or implicitly by not making tinderbox compare the modules (i.e., extensions/spellcheck)
Attachment #280079 - Flags: review?(benjamin)
Comment on attachment 280077 [details] [diff] [review]
call out to $(app)/locales/filter.py for exceptions

this looks like it should work and is a bit simpler from the previous version. Dynamically loading callables makes the source a tad unreadable, but that's what you get for your convenience.
Attachment #280077 - Flags: review?(rcampbell) → review+
Comment on attachment 280079 [details] [diff] [review]
exceptions for Firefox and Thunderbird

>Index: browser/locales/filter.py

>+  if mod not in ["netwerk", "dom", "toolkit", "security/manager",
>+                 "browser", "extensions/reporter", "extensions/spellcheck",
>+                 "other-licenses/branding/firefox"]:

nit, using a (tuple) performs slightly better than a [list]

Several people have suggested that regexes should always be r"" raw strings to avoid errors with multiple escapes.

r=me with those changes
Attachment #280079 - Flags: review?(benjamin) → review+
Comment on attachment 280079 [details] [diff] [review]
exceptions for Firefox and Thunderbird

Self-approving the exception files to land on the branch, no real impact there as they're not used by tinderbox. They are used by the 1.8 status page though, which is already tested locally.

The python changes don't need branch landing, the trunk tools work for the branch.
Attachment #280079 - Flags: approval1.8.1.8+
FIXED on trunk and branch, verified, too. Thanks for the reviews.

compare-locales should be usable for suite and calendar applications on the trunk now.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: verified1.8.1.8
Resolution: --- → FIXED

Updated

10 years ago
Blocks: 403993
Component: Testing → Infrastructure
Product: Core → Mozilla Localizations
QA Contact: testing → infrastructure
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.