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

Status: RESOLVED FIXED

(Mozilla Localizations :: Infrastructure, defect)

Description

[Axel Hecht [:Pike]]

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?

Comment 1

[Axel Hecht [:Pike]]

Attachment: call out to $(app)/locales/filter.py for exceptions

Comment 2

[Axel Hecht [:Pike]]

Attachment: 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)

Comment 3

[Rob Campbell [:rc] (:robcee)]

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.

Comment 4

[Benjamin Smedberg]

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

Comment 5

[Axel Hecht [:Pike]]

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.

Comment 6

[Axel Hecht [:Pike]]

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.