Closed Bug 1509391 Opened 3 years ago Closed 3 years ago
eslint linter should automatically ignore stuff from tools/rewriting/Third
Party Paths .txt
47 bytes, text/x-phabricator-request
|Details | Review|
Looks like eslint has its own ~/.eslintignore file for folders to skip linting on. It should probably also skip all the things in tools/rewriting/ThirdPartyPaths.txt - currently it does not.
Note that mozlint does read this file globally already: https://searchfox.org/mozilla-central/source/tools/lint/mach_commands.py If possible mozlint will try to filter these out automatically, but in some cases it can't so relies on the underlying implementation to do so. In this case these paths can all be accessed via `config['exclude']` here: https://searchfox.org/mozilla-central/source/tools/lint/eslint/__init__.py#45 It looks like eslint has a --ignore-pattern argument which we should be able to forward those into.
This is pretty much a duplicate of bug 1366714 - at least until we decide to move to dedicated lint plugins for editors. I guess we could do this, but we should determine if we're going to upset developers by moving third party stuff out of the ignore file.
Normally eslint handles its own file exclusions, but there are still some globally excluded paths that |mach lint| passes in (e.g objdirs and things in ThirdPartyPaths.txt). This makes sure that if they show up in the 'config', we handle them.
The above fixes it. Though a downside is that it'll make the eslint command longer which might make it easier for Windows users to hit the max command length.
Feel free to push back on this if you had concerns.
(In reply to Mark Banner (:standard8) from comment #2) > I guess we could do this, but we should determine if we're going to upset > developers by moving third party stuff out of the ignore file. If that's actually a sticking point (though I'm not sure why it would be), then there's no harm in using the paths from ThirdPartyPaths.txt as well as leaving them in .eslintignore. But I think it's important we have a single source of truth for finding third party paths, rather than maintain a separate duplicate copy in .eslintignore.
I guess your concern is more centered with the config used by editor integrations, in which case I should clarify: I don't care if we duplicate 3rd party stuff in .eslintignore, as long as we *also* apply the stuff in ThirdPartyPaths.txt so there's no chance of things getting out of sync.
Ok, I think this is fine for now, though we need to figure out the longer term future of how we manage all of this.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/c8e417bfccfb [eslint] Ignore excluded files that mozlint wasn't able to handle automatically r=Standard8
Backed out changeset c8e417bfccfb (bug 1509391) for build bustage. CLOSED TREE Log: https://treeherder.mozilla.org/logviewer.html#?job_id=213951548&repo=autoland&lineNumber=50358 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=c8e417bfccfbba8bd7917368778939949d1126bd&selectedJob=213951548 Backout: https://hg.mozilla.org/integration/autoland/rev/0a2f54a1a5ca4e1a14ed516fe4b7de2bf8d23a05
Sorry, forgot to run a Windows build in my try run (hopefully I'll be able to land bug 1436037 soon and we'll be able to finally pull these tasks out of the build jobs). Anyway, looks like this failed because we can't find node on the build workers which isn't terribly surprising. Seeing as we'll be porting these tasks to a new workertype soon, I don't want to spend effort solving this. So I'm just going to skip this new test on Windows for now and leave enabling it on Windows as a follow-up.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/a823c2262dbf [eslint] Ignore excluded files that mozlint wasn't able to handle automatically r=Standard8
You need to log in before you can comment on or make changes to this bug.