Closed Bug 1426882 Opened 3 years ago Closed 2 years ago
set up eslint rules in /mailnews xpcshell tests
+++ This bug was initially created as a clone of Bug #1426459 +++ There are already predefined rules for xpcshell tests at mozilla/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/xpcshell-test.js, mainly silencing warnings about xpcshell global functions whose declarations can't be seen by the linter. Let's use it for mailnews tests.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f82a79dda31beaefb1387ec755b11ef9f3aa4e95
Attachment #8938676 - Flags: review?(philipp)
Comment on attachment 8938676 [details] [diff] [review] 1426882.patch Review of attachment 8938676 [details] [diff] [review]: ----------------------------------------------------------------- This is generally ok so r+, but please only land this if the lint target remains green and the directories you added .eslintrc.js to are actually included in the lint run.
Attachment #8938676 - Flags: review?(philipp) → review+
Well, none of that is true now and can hardly be satisfied. This is meant to be a baseline setting on which further bugs cleaning up individual tests/directories can build.
In that case I think it makes more sense to add this once the directories are cleaned. There is no use adding this config if the directories are not run. Possibly WONTFIX and then open more specific bugs to clean up the test directories?
For me it does not make sense. Anybody (including me) in the future will have to wonder about the many useless warnings, remember to include this predefined file and only then start to fix real problems. For me it makes more sense to land this when we already thought about this and save time in the future.
So then maybe turn it around: create an /mailnews/.eslintrc.js that disables all failing rules explicitly, with a comment that these are to be enabled later, then remove mailnews from eslintignore. This way the directory is run, and you can enable the remaining rules gradually. Given we disagree about the approach I'm happy to listen to a third opinion though.
I think putting FIXME's in the lint configuration is the right solution. That is what is done other places: https://dxr.mozilla.org/mozilla-central/source/taskcluster/.yamllint#10
Geoff, could you finish this when you get some time?
Eventually. When I've finished mail/components I'm aiming to do mail/base. Maybe after that?
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1515877
You need to log in before you can comment on or make changes to this bug.