Closed Bug 1288425 Opened 4 years ago Closed 4 years ago
[mozlint] Flake8 will attempt to lint non-python files if you explicitly pass them in
58 bytes, text/x-review-board-request
STR: mach lint -l flake8 tools/lint/eslint-formatter.js If you explicitly pass in a file to flake8, it will disregard the 'filenames' config. I think this is correct behaviour on flake8's part, but we need to handle this in mozlint because this situation will be common when using the --rev/--workdir options (those will explicitly pass in every file that was touched, which probably include non-python files). Ftr, I noticed that eslint will also spew out a warning (that we should avoid) if you explicitly pass it a .py file. So mozlint will need to be able to handle file extensions. For now I'll add it to the existing LINTER dictionary. But I'm getting more and more convinced that I should come up with a better configuration system.
Some linters, such as flake8, will lint invalid file extensions if you explicitly pass them in. E.g, |flake8 foobar.js| will result in flake8 attempting to lint a JS file. This is a problem because passing in files explicitly is exactly what the --rev/--workdir options do. If a developer modifies a JS file then runs |mach lint -l flake8 -w|, that JS file will get linted. To prevent this, mozlint needs to handle file extensions instead of relying on the underlying linter to do it. This patch adds an "extensions" config option to the LINTER dict, and will filter these files out as part of the 'filterpaths' steps. Review commit: https://reviewboard.mozilla.org/r/66054/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/66054/
Attachment #8773311 - Flags: review?(smacleod)
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment on attachment 8773311 [details] Bug 1288425 - Make sure we skip invalid extensions when linting with --rev or --workdir, https://reviewboard.mozilla.org/r/66054/#review64164
Attachment #8773311 - Flags: review?(smacleod) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/2141360b4137 Make sure we skip invalid extensions when linting with --rev or --workdir, r=smacleod
sorry had to backout your push for build bustage, e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=1014985&repo=autoland#L30587
Backout by email@example.com: https://hg.mozilla.org/integration/autoland/rev/2f3e55cbfea9 Backed out changeset 2141360b4137 for build bustage
Sorry, bit of a perfect storm. These tests: A) Pass for me locally with just this commit B) Pass on try when included with another patch I wrote C) Fail on try with just this commit alone I was originally going to file a new bug for the other commit in the series, but given that this commit depends on it, I'll upload it here shortly instead.
Nvm, mozreview doesn't like me re-submitting a new commit to the old review. So I'll wait for bug 1289805 to land before re-landing this (after proper try coverage of course!)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/23e3bb6e8e93 Make sure we skip invalid extensions when linting with --rev or --workdir, r=smacleod
This is causing build oranges on c-c, due to mozlint test failures e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=44597&repo=comm-central#L28848-L29854 I suspect that's due to the path in https://hg.mozilla.org/mozilla-central/rev/23e3bb6e8e93#l3.14 (which might have to be mozilla/python/... for comm builds)?
Sorry, yeah that looks like the reason. The mozlint tests definitely shouldn't depend on where they're located within the tree. I have a fix, and will file a new bug to clean this up.
(In reply to Andrew Halberstadt [:ahal] from comment #12) > Sorry, yeah that looks like the reason. The mozlint tests definitely > shouldn't depend on where they're located within the tree. I have a fix, and > will file a new bug to clean this up. Thanks!
You need to log in before you can comment on or make changes to this bug.