Closed
Bug 1473308
Opened 6 years ago
Closed 6 years ago
"./mach lint" is applying flake8 to ".configure" files and finding issues that are not existing
Categories
(Developer Infrastructure :: Lint and Formatting, defect)
Developer Infrastructure
Lint and Formatting
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: marco, Assigned: ahal)
References
Details
Attachments
(1 file)
For example if you run "./mach lint mobile/android/gradle.configure", a lot of problems will be found (e.g. "set_config" not being defined). The static analysis bot is then complaining about this, for example: https://bugzilla.mozilla.org/show_bug.cgi?id=1471660#c4.
Updated•6 years ago
|
Flags: needinfo?(ahal)
Assignee | ||
Comment 2•6 years ago
|
||
There's a bug in our flake8 integration. We're supposed to be linting ".configure" files, but aren't passing .configure as a valid file extension down to the flake8 binary. However, if you pass a file directly into flake8, it'll lint it regardless of its extension. So: ./mach lint -l flake8 mobile/android/gradle.configure Will cause it to be linted (due to flake8 not caring about its extension). Whereas: ./mach lint -l flake8 mobile/android Will not lint the file since flake8 doesn't know to lint the .configure extension. So I'll need to pass in that argument and exclude all the .configure files with errors in them until we can fix them up.
Assignee: nobody → ahal
Status: NEW → ASSIGNED
Flags: needinfo?(ahal)
Assignee | ||
Comment 3•6 years ago
|
||
We're supposed to be linting both .py and .configure files with flake8. However we never inform flake8 of this fact. So e.g running: ./mach lint -l flake8 mobile/android Will not lint mobile/android/gradle.configure. However since flake8 will run on a file regardless of its extension if you pass that file in directly, it means that running: ./mach lint -l flake8 mobile/android/gradle.configure *Will* cause the file to be linted (and subsequently fail). This fix makes sure that flake8 knows to look at .configure files in addition to .py. Since this means many .configure files around the tree will start getting linted for the first time, we need to exclude them until they can be fixed.
Comment 4•6 years ago
|
||
Comment on attachment 8990086 [details] Bug 1473308 - [flake8] Pass custom extension list into the flake8 binary Marco Castelluccio [:marco] has approved the revision. https://phabricator.services.mozilla.com/D1975
Attachment #8990086 -
Flags: review+
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b9e9d507a53c [flake8] Pass custom extension list into the flake8 binary r=marco
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b9e9d507a53c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 7•6 years ago
|
||
Andrew, could you please report a bug so that we make *.configure compliant? So that we don't forget. Thanks! (maybe as a good first bug)
Flags: needinfo?(ahal)
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•