Closed Bug 1425280 Opened 2 years ago Closed 2 years ago

Fix localization note mismatch in devtools

Categories

(DevTools :: General, enhancement, P3)

58 Branch
enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: jdescottes, Assigned: jdescottes)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

I would like to have a test similar to browser_misused_characters_in_strings.js but focused on catching localization note issues. Initially I thought I would only add it for devtools, but that might be interesting for m-c so let me know what you think.

The test itself is not very strict. It only checks that when there _is_ a localization note:
- it matches a definition in the dtd/properties file
- it is not duplicated with another localization note (which is usually what happens when you copy paste another string)
Attachment #8936967 - Flags: feedback?(francesco.lodolo)
Attachment #8936967 - Flags: feedback?(florian)
Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

The idea sounds good (gosh, so many errors), but I'd like to hear Pike's thoughts on the actual implementation, where I can't really be that useful.
Attachment #8936967 - Flags: feedback?(francesco.lodolo) → feedback?(l10n)
Comment on attachment 8936967 [details]
Bug 1425280 - add a test to catch localization note mismatch

https://reviewboard.mozilla.org/r/207704/#review213620

In bug 1353680, I'm adding a linter for l10n, based on existing parsers. I'm actually rebasing that patch these days, so it's not that that bug is as stale as it looks.

This test should be part of that infrastructure, and I really don't want us to have more variants of trying to parse our l10n files.

Thus I'm giving this an r-, as much as this obviously helped you to find bugs.
Attachment #8936967 - Flags: review-
Attachment #8936967 - Flags: feedback?(l10n)
I will block this on your linter bug and will revisit once the linter infrastructure is ready.
Depends on: 1353680
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Attachment #8936967 - Attachment is obsolete: true
Attachment #8936967 - Flags: feedback?(florian)
Repurposing to land the devtools-only fixes.
Summary: Add test to catch localization note mismatch → Fix localization note mismatch in devtools
Attachment #8936969 - Attachment is obsolete: true
Comment on attachment 8936968 [details]
Bug 1425280 - Fix localization note mismatch in devtools files;

https://reviewboard.mozilla.org/r/207706/#review215400

Thank you for catching and fixing all of these inconsistencies Julian.
Attachment #8936968 - Flags: review?(pbrosset) → review+
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Thanks for the review! I'll keep monitoring bug 1353680, hopefully we can have a linter in the future to avoid reintroducing this kind of issues.
No longer depends on: 1353680
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/889f51eea06e
Fix localization note mismatch in devtools files;r=pbro
https://hg.mozilla.org/mozilla-central/rev/889f51eea06e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
See Also: → 1452187
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.