New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run flake8
on more files
#85333
Run flake8
on more files
#85333
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great addition! 👏
lgtm cc @asottile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seem to be lots of unrelated changes in this PR. Several of the function description changes seem to have typos. Maybe they could be moved to a separate PR.
@davet2001 all these changes are to satisfy the flake8 errors. Without them, the errors are:
|
Sorry, I misunderstood some of these edits. Only minor tweaks needed I think. :) |
No problem, will get the fixes out shortly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ok to me now. The question still remains whether it is wanted to add flake8 restrictions to these parts of the repo. I assume yes because it will tend to improve quality/consistency.
I agree as well. Are you able to merge this or do we need someone else to make a final call? |
@frenck please could we get your opinion on increasing flake8 scope in this way? |
Hi @frenck just wondering if you had a moment to look per @davet2001's comment, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks fine, thanks @mxr 👍
../Frenck
Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
Proposed change
In order to hold more of the code to a high standard, this PR enables
flake8
on all Python files, except the auto-generated sphinx config file. This PR uses a denylist instead of an allowlist so new folders/files will get linted without requiring developers to change the linter targeting rules.Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: