1478613 - compare_locales.paths could use a split

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: compare-locales, enhancement)

Description

[Axel Hecht [:Pike]]

compare_locales.paths has become quite big and unwieldy, let's split that up a little.

I've got a couple of things I'd like to change there, doing the refactor first will be nice.

I'll take some minor fixups of other wip branches along for the ride, 'cause they'd be naughty to rebase.

Comment 1

[Axel Hecht [:Pike]]

Attachment: bug 1478613, expose remaining variables in matcher regex,

If we match a path, we'd like to know which variables got which
value. Say, the {locale} code in a path.

Review commit: https://reviewboard.mozilla.org/r/259720/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259720/

Comment 2

[Axel Hecht [:Pike]]

Attachment: bug 1478613, extract path variable expansion into Matcher,

Review commit: https://reviewboard.mozilla.org/r/259722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259722/

Comment 3

[Axel Hecht [:Pike]]

Attachment: bug 1478613, make paths a module,

Review commit: https://reviewboard.mozilla.org/r/259724/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259724/

Comment 4

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor paths.Matcher,

Review commit: https://reviewboard.mozilla.org/r/259726/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259726/

Comment 5

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor paths.ProjectConfig,

Review commit: https://reviewboard.mozilla.org/r/259728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259728/

Comment 6

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor ini config loading classes,

When we drop ini configs finally, we should be able to
remove this file completely.

Review commit: https://reviewboard.mozilla.org/r/259730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259730/

Comment 7

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor paths.ProjectFiles,

Review commit: https://reviewboard.mozilla.org/r/259732/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259732/

Comment 8

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor paths.TOMLParser,

Review commit: https://reviewboard.mozilla.org/r/259734/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259734/

Comment 9

[Axel Hecht [:Pike]]

Attachment: bug 1478613, refactor tests for compare_locales.paths,

Review commit: https://reviewboard.mozilla.org/r/259736/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259736/

Comment 10

[Staś Małolepszy :stas]

Comment on attachment 8995243 [details]
bug 1478613, expose remaining variables in matcher regex,

https://reviewboard.mozilla.org/r/259720/#review266922

Comment 11

[Staś Małolepszy :stas]

Comment on attachment 8995244 [details]
bug 1478613, extract path variable expansion into Matcher,

https://reviewboard.mozilla.org/r/259722/#review266924

Comment 12

[Staś Małolepszy :stas]

Comment on attachment 8995245 [details]
bug 1478613, make paths a module,

https://reviewboard.mozilla.org/r/259724/#review266926

Comment 13

[Staś Małolepszy :stas]

Comment on attachment 8995246 [details]
bug 1478613, refactor paths.Matcher,

https://reviewboard.mozilla.org/r/259726/#review266928

Comment 14

[Staś Małolepszy :stas]

Comment on attachment 8995247 [details]
bug 1478613, refactor paths.ProjectConfig,

https://reviewboard.mozilla.org/r/259728/#review266930

Comment 15

[Staś Małolepszy :stas]

Comment on attachment 8995248 [details]
bug 1478613, refactor ini config loading classes,

https://reviewboard.mozilla.org/r/259730/#review266932

Comment 16

[Staś Małolepszy :stas]

Comment on attachment 8995249 [details]
bug 1478613, refactor paths.ProjectFiles,

https://reviewboard.mozilla.org/r/259732/#review266934

Comment 17

[Staś Małolepszy :stas]

Comment on attachment 8995250 [details]
bug 1478613, refactor paths.TOMLParser,

https://reviewboard.mozilla.org/r/259734/#review266936

Comment 18

[Staś Małolepszy :stas]

Comment on attachment 8995251 [details]
bug 1478613, refactor tests for compare_locales.paths,

https://reviewboard.mozilla.org/r/259736/#review266938

Comment 19

[Staś Małolepszy :stas]

As far as I could tell, these changes look good. That said, I'm not familiar with this code and I'd prefer not to be on the review path for it. In the future, please feel free to land without a review. If you'd still like another pair of eyes to glance over the patch (which makes sense), please submit a single patch. I don't see much value in having a queue this long for a change that is semantically atomic. Thanks!

Comment 20

[Axel Hecht [:Pike]]

https://hg.mozilla.org/l10n/compare-locales/pushloghtml?changeset=81145ff9253f, landed.