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



12 years ago
9 years ago


(Reporter: Pike, Assigned: Pike)



Firefox Tracking Flags

(Not tracked)



(2 attachments)



12 years ago
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 I just dropped regular expressions where I don't need them.

In, 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

12 years ago
Assignee: nobody → l10n
Attachment #280077 - Flags: review?(rcampbell)

Comment 2

12 years ago
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/ 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 4

12 years ago
Comment on attachment 280079 [details] [diff] [review]
exceptions for Firefox and Thunderbird

>Index: browser/locales/

>+  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 5

12 years ago
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+

Comment 6

12 years ago
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.
Closed: 12 years ago
Keywords: verified1.8.1.8
Resolution: --- → FIXED


12 years ago
Blocks: 403993


11 years ago
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.