1813186 - Add lint rules for newly added string resources

Status: RESOLVED FIXED

(Fenix :: General, task, P3)

Description

[Jeff Boek [:boek]]

From github: https://github.com/mozilla-mobile/fenix/issues/7210.

I know very little of the current technical infrastructure, but I couldn't help noticing that sometimes strings land with the wrong apostrophe, and that's manually caught by Delphine during review.

Would it be possible to automate that kind of checks via tests?

CC @Delphine @Pike

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Jeff Boek [:boek]]

Since you assigned yourself to to https://github.com/mozilla-mobile/fenix/issues/26645 I thought I would tag you here so you wouldn't lose it :)

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Thanks! This is very much appreciated.

Comment 3

[Joe M [:jmahon]]

I'm trying to remember why I made this a P1; I think it was because I assumed it was actively in progress.

:gl, is that accurate? Or should we move this to a lower priority? (and drop it from the "up next" column in our kanban board?)

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Joe M [:jmahon] from comment #3)

I'm trying to remember why I made this a P1; I think it was because I assumed it was actively in progress.

:gl, is that accurate? Or should we move this to a lower priority? (and drop it from the "up next" column in our kanban board?)

This should be a much lower priority. I felt like you moved this into the board simply because it was assigned, but beyond that this is not urgent.

Comment 5

[Francesco Lodolo [:flod]]

This is the linter I'm currently using in android-l10n.

Checks:

• Typography: warn when ', ", and ... (instead of …) are used.
• Hard-coded brand names (provided in this config).

For other formats, I have checks that force a comment to include references to the variables in the string, to make sure the developer explains what replaces the variables, i.e.

• Parse a string for placeables (%s, %1$s, etc.).
• Check the comment for the same placeables.

All these checks require a list of exceptions, to work around actual exceptions (e.g. HTML code with straight quotes) or just legacy strings.

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

This ticket is about adding a linter to catch all the issues that comes up with adding new string resources.

For the monorepo migration, we will be introducing a bit of a delay before strings are exported to the android-l10n repository. Strings need to make their way to gecko-dev before they are extracted via a GitHub actions to the android-l10n repository. This means that strings need to land in autoland, make their way into central and then synced into the GitHub mirror before they are exported to the android-l10n repo.

As a result, we want to have a linter in place to catch problems earlier to reduce the number of issues that will be caught days later.

Comment 7

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1813186 - Part 1: Add lint rules for string resources

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1813186 - Part 2: Adds string lint ignores to exclude imported l10n paths and static string files

• Add string lint ignores to exclude imported l10n paths and static string files in Fenix, Focus and Android Components.

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1813186 - Part 3: Use the correct quotation character for existing strings

• This fixes some of the incorrect quotation character used in our existing strings.
• Currently, we do have an ignore list in the android-l10n repo for any of the existing exceptions https://github.com/mozilla-l10n/android-l10n/blob/main/.github/scripts/linter_config.json

Comment 10

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1813186 - Part 4: Suppress string lint errors for existing strings that are part of the exception

• Suppresses string lint errors that are already part of the exception list found below.
• These are brand name errors that are hard-coded due to their usage in Nimbus and quotation character errors that are used in HTML tags.
• Currently, we do have an exception list in the android-l10n repo for any of the existing exceptions https://github.com/mozilla-l10n/android-l10n/blob/main/.github/scripts/linter_config.json

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a87614b564f6
Part 1: Add lint rules for string resources r=android-reviewers,tthibaud
https://hg.mozilla.org/integration/autoland/rev/5dffc3ff5ecb
Part 2: Adds string lint ignores to exclude imported l10n paths and static string files r=android-reviewers,tthibaud
https://hg.mozilla.org/integration/autoland/rev/ac6fbfd54594
Part 3: Use the correct quotation character for existing strings r=android-reviewers,tthibaud
https://hg.mozilla.org/integration/autoland/rev/6b0ece9382f6
Part 4: Suppress string lint errors for existing strings that are part of the exception r=android-reviewers,tthibaud

Comment 12

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/a87614b564f6
https://hg.mozilla.org/mozilla-central/rev/5dffc3ff5ecb
https://hg.mozilla.org/mozilla-central/rev/ac6fbfd54594
https://hg.mozilla.org/mozilla-central/rev/6b0ece9382f6