1385234 - [compare-locales] entity list in getChecks should be optional, and late

Status: RESOLVED FIXED

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

Description

[Axel Hecht [:Pike]]

When using the compare-locales checks in pontoon, it'd be nice to know when we have to provide the list of entities to the checks.

Right now, that's hard-coded in compare-locales in a weird way.

Let's move that logic to the Checker, and add entities on demand.

Comment 1

[Axel Hecht [:Pike]]

Attachment: bug 1385234, add reference entity list to checkers after creation,

Switch the API on getChecker to allow the reference entity list to be
avoided. Instead add a flag on the Checker class that asks for it
to be set.
This allows external users to inspect this, and use compare-locales
checks more efficiently.

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

Comment 2

[u599210]

Comment on attachment 8891305 [details]
bug 1385234, add reference entity list to checkers after creation,

https://reviewboard.mozilla.org/r/162510/#review167754

::: compare_locales/compare.py:472
(Diff revision 1)
>          report = missing = obsolete = changed = unchanged = keys = 0
>          missing_w = changed_w = unchanged_w = 0  # word stats
>          missings = []
>          skips = []
> -        checker = getChecker(l10n,
> -                             reference=ref_entities, extra_tests=extra_tests)
> +        checker = getChecker(l10n, extra_tests=extra_tests)
> +        if checker and checker.needs_reference:

I have a small nitty observation: what will happen if e.g. programmer will forget to execute set_reference for DTD/similar classes? I'm just not sure if we should notify developer by raising an exception or just leave it and assume the default behaviour. I'm not sure how many projects use compare-locales.

Comment 3

[Axel Hecht [:Pike]]

(In reply to poke+github from comment #2)
> Comment on attachment 8891305 [details]
> I have a small nitty observation: what will happen if e.g. programmer will
> forget to execute set_reference for DTD/similar classes? I'm just not sure
> if we should notify developer by raising an exception or just leave it and
> assume the default behaviour. I'm not sure how many projects use
> compare-locales.

I've actually tried that, by accident. I forgot to set .needs_reference on the DTDChecker, and nothing crashed or so. It just returned more warnings.

And, with pontoon, we'd have two projects using it, compare-locales and pontoon ;-)

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 8891305 [details]
bug 1385234, add reference entity list to checkers after creation,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162510/diff/1-2/

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 8891305 [details]
bug 1385234, add reference entity list to checkers after creation,

https://reviewboard.mozilla.org/r/162510/#review167836

Carrying over r+ from jotes on the final patch (just had to fix commit message).

Comment 6

[Axel Hecht [:Pike]]

https://hg.mozilla.org/l10n/compare-locales/rev/c36e2fd09e8282431a933afed44969fbc09bb418, marking FIXED.