Closed
Bug 1428769
Opened 7 years ago
Closed 7 years ago
intl/l10n should be covered by eslint
Categories
(Core :: Internationalization, defect, P3)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: Pike, Assigned: zbraniecki)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
Right now, all of intl/** is excluded from eslint, via the top-level .eslintignore.
We should fix that at least for the code we write newly in intl/l10n. The linter failures in intl/l10n/test are already pretty lengthy, with an intl/l10n/test/.eslintrc of
"use strict";
module.exports = {
"extends": "plugin:mozilla/xpcshell-test"
};
Bug 1426063 adds more, so filing this.
Assignee | ||
Comment 1•7 years ago
|
||
Thanks for filing this Axel! I think it's blocked by bug 1393953 now. I was hoping to work on that cleanup after 0.5 release.
Depends on: 1393953
Updated•7 years ago
|
Priority: -- → P3
Reporter | ||
Comment 2•7 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #1)
> Thanks for filing this Axel! I think it's blocked by bug 1393953 now. I was
> hoping to work on that cleanup after 0.5 release.
I agree that that's necessary to lint all of intl/l10n, but that doesn't affect the code that's only in mozilla-central, like integration code and tests.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
This is 99% https://github.com/projectfluent/fluent.js/commit/8e4f284e04b9333393da8c66dca46d82f3e60b57 and 1% test_l10nregistry.js alignment. :)
Depends on bug 1439949 to get async-iterators support in eslint.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8952737 [details]
Bug 1428769 - Add intl/l10n to be covered by eslint.
https://reviewboard.mozilla.org/r/221970/#review227834
Attachment #8952737 -
Flags: review?(l10n) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
After bug 1439949 landed, we got some more eslint errors, which I just autofixed. I'll need to sync that back to fluent-gecko which will happen in bug 1434434.
Comment 9•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 5c07bdfad5c24905680fddbb1abea0da9c157a80 -d a94434a6f077: rebasing 448327:5c07bdfad5c2 "Bug 1428769 - Add intl/l10n to be covered by eslint. r=Pike" (tip)
merging intl/l10n/L10nRegistry.jsm
merging intl/l10n/Localization.jsm
warning: conflicts while merging intl/l10n/L10nRegistry.jsm! (edit, then use 'hg resolve --mark')
warning: conflicts while merging intl/l10n/Localization.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a34dd4ed3e3
Add intl/l10n to be covered by eslint. r=Pike
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8952737 [details]
Bug 1428769 - Add intl/l10n to be covered by eslint.
https://reviewboard.mozilla.org/r/221970/#review228098
Code analysis found 2 defects in this patch:
- 2 defects found by mozlint
You can run this analysis locally with:
- `./mach lint path/to/file` (JS/Python)
If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx
::: intl/l10n/Localization.jsm:256
(Diff revision 4)
>
> /**
> * Register weak observers on events that will trigger cache invalidation
> */
> registerObservers() {
> - ObserverService.addObserver(this, 'intl:app-locales-changed', true);
> + Services.obs.addObserver(this, 'intl:app-locales-changed', true);
Error: Strings must use doublequote. [eslint: quotes]
::: intl/l10n/Localization.jsm:263
(Diff revision 4)
>
> /**
> * Unregister observers on events that will trigger cache invalidation
> */
> unregisterObservers() {
> - ObserverService.removeObserver(this, 'intl:app-locales-changed');
> + Services.obs.removeObserver(this, 'intl:app-locales-changed');
Error: Strings must use doublequote. [eslint: quotes]
Comment 13•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•