Closed Bug 1426882 Opened 3 years ago Closed 2 years ago

set up eslint rules in /mailnews xpcshell tests

Categories

(Thunderbird :: Build Config, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1515877

People

(Reporter: aceman, Assigned: aceman)

References

Details

Attachments

(1 file)

+++ 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.
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?
Flags: needinfo?(geoff)
Eventually. When I've finished mail/components I'm aiming to do mail/base. Maybe after that?
Flags: needinfo?(geoff)
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.