Closed Bug 1428769 Opened 7 years ago Closed 7 years ago

intl/l10n should be covered by eslint

Categories

(Core :: Internationalization, defect, P3)

defect

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.
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
Priority: -- → P3
(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.
Depends on: 1439949
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
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.
Attachment #8952737 - Flags: review?(l10n) → review+
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.
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)
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2a34dd4ed3e3 Add intl/l10n to be covered by eslint. r=Pike
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]
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: