Closed Bug 1623024 Opened 4 months ago Closed 26 days ago

mozlint: Add pylint as a new linter

Categories

(Firefox Build System :: Lint and Formatting, defect)

defect
Not set
normal

Tracking

(firefox79 fixed)

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=python])

Attachments

(4 files)

it finds issues that flake8 doesn't find
For example, the missing coma here:
https://searchfox.org/mozilla-central/source/dom/bindings/parser/WebIDL.py#3483-3484
https://searchfox.org/mozilla-central/source/dom/bindings/parser/WebIDL.py#5470-5473

dom/bindings/parser/WebIDL.py:3428:26: E1120: No value for argument 'locations' in constructor call (no-value-for-parameter)
dom/bindings/parser/WebIDL.py:5430:22: E1120: No value for argument 'locations' in constructor call (no-value-for-parameter)

To evaluate it, we should run it on directory on which we are running flake8 already. See if it does find interesting stuff
https://searchfox.org/mozilla-central/source/.flake8 (the exclude list)

Then we could add a new linter:
https://firefox-source-docs.mozilla.org/code-quality/lint/create.html

Adding a good first bug which is probably good second bug for someone who knows python already

Mentor: sledru
Keywords: good-first-bug
Whiteboard: [lang=python]
Blocks: 1623061

IMO pylint is way too strict by default, we'd need to be very conscious of the rules we enable. Ideally we wouldn't want overlap between pylint / flake8 either so devs don't see duplicated issues.

Yeah, we should enable it on stuff that flake8 doesn't cover (at least no-value-for-parameter)

Here is what code is doing:
https://code.visualstudio.com/docs/python/linting#_pylint

Should be pretty easy to add.

Assignee: nobody → sledru
Attachment #9155573 - Attachment description: Bug 1623024 - mozlint: Add pylint as new linter → Bug 1623024 - mozlint: Add pylint as new linter r?#linter-reviewers
Attachment #9155604 - Attachment description: Bug 1623024 - mozlint: Add pylint to the CI → Bug 1623024 - mozlint: Add pylint to the CI r?#linter-reviewers
Attachment #9156209 - Attachment description: Bug 1623024 - mozlint: run pylint at review phase → Bug 1623024 - mozlint: run pylint at review phase r?#linter-reviewers
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba8087e38e6c
mozlint: Add pylint as new linter r=linter-reviewers,ahal
https://hg.mozilla.org/integration/autoland/rev/2ba0929ca2b5
mozlint: Add pylint to the CI r=linter-reviewers,ahal
https://hg.mozilla.org/integration/autoland/rev/ef81294f5db7
mozlint: run pylint at review phase r=linter-reviewers,ahal
Status: NEW → RESOLVED
Closed: 26 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e16a500ce946
mozlint/pylint - follow up: fix a syntax issue in the rst file
Depends on: 1647254
Alias: pylint
Summary: mozlint: Evaluate running pylint on the code base → mozlint: Add pylint as a new linter
Depends on: 1647259
Depends on: 1647260
No longer depends on: 1647254, 1647259, 1647260
See Also: → pylint
Alias: pylint
Type: enhancement → defect
No longer blocks: 1648586
Regressions: 1648586
You need to log in before you can comment on or make changes to this bug.