Skip to content
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

Add a flake8 command to etstool.py #1027

Merged
merged 2 commits into from
Sep 28, 2021
Merged

Conversation

corranwebster
Copy link
Contributor

This PR also adds a setup.cfg which makes flake8 run cleanly. There are
things in the clean list that are likely errors and need to be fixed.

This doesn't add flake8 to CI, that should be a separate PR.

This PR also adds a setup.cfg which makes flake8 run cleanly.  There are
things in the clean list that are likely errors and need to be fixed.

This doesn't add flake8 to CI, that should be a separate PR.
@corranwebster corranwebster added this to the Release 7.4.0 milestone Sep 28, 2021
parameters = get_parameters(edm, runtime, toolkit, environment)
config = ""
if strict:
config = "--config=flake8_strict.cfg "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what this file is. should this be setup.cfg?

Also, I don't fully understand what the point of "strict" vs "non-strict" options for flake8 is - don't we always want to use "strict"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy-pasta from TraitsUI - the idea was strict is something like what we are aiming for (and so new code should pass strict), but there are a lot of issues that need fixing before old code could pass the strict version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously I'll fix one way or another.

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for now. We'll need to come back and make changes anyway as we slowly address the flake8 issues.

@corranwebster corranwebster merged commit e40adda into main Sep 28, 2021
@corranwebster corranwebster deleted the enh/add-flake8-command branch September 28, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants