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)

defect
Not set
normal

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.
Flags: needinfo?(ahal)
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)
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 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
https://hg.mozilla.org/mozilla-central/rev/b9e9d507a53c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
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)
Blocks: 1478338
Bug 1478338
Flags: needinfo?(ahal)
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.