Last Comment Bug 395396 - Move exceptions from compare-locales to $(app)/locales
: Move exceptions from compare-locales to $(app)/locales
Status: RESOLVED FIXED
: verified1.8.1.8
Product: Mozilla Localizations
Classification: Client Software
Component: Infrastructure (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Axel Hecht [:Pike]
:
Mentors:
Depends on:
Blocks: 403993
  Show dependency treegraph
 
Reported: 2007-09-07 09:57 PDT by Axel Hecht [:Pike]
Modified: 2010-10-19 10:36 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
call out to $(app)/locales/filter.py for exceptions (5.87 KB, patch)
2007-09-07 09:59 PDT, Axel Hecht [:Pike]
rcampbell: review+
Details | Diff | Splinter Review
exceptions for Firefox and Thunderbird (2.19 KB, patch)
2007-09-07 10:01 PDT, Axel Hecht [:Pike]
benjamin: review+
l10n: approval1.8.1.8+
Details | Diff | Splinter Review

Description Axel Hecht [:Pike] 2007-09-07 09:57:21 PDT
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] 2007-09-07 09:59:00 PDT
Created attachment 280077 [details] [diff] [review]
call out to $(app)/locales/filter.py for exceptions
Comment 2 Axel Hecht [:Pike] 2007-09-07 10:01:23 PDT
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)
Comment 3 Rob Campbell [:rc] (:robcee) 2007-09-13 10:08:24 PDT
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 [:bsmedberg] 2007-09-17 19:58:29 PDT
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] 2007-09-20 03:41:05 PDT
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] 2007-09-20 05:32:21 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.