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.

Sign up for GitHub

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

Jump to bottom

Apply flake8 suggested fixes and add flake8 GitHub CI action with black compatible config #29

Closed
johanneskastl opened this issue Mar 15, 2023 · 4 comments
Closed

Apply flake8 suggested fixes and add flake8 GitHub CI action with black compatible config #29

johanneskastl opened this issue Mar 15, 2023 · 4 comments

Comments

@johanneskastl
Copy link

As I prepared a PR today and wanted to make sure I do not introduce errors, I linted the go_modules file using flake8.

flake8 go_modules
go_modules:74:80: E501 line too long (88 > 79 characters)
go_modules:87:63: W605 invalid escape sequence '\.'
go_modules:87:80: E501 line too long (101 > 79 characters)
go_modules:87:90: W605 invalid escape sequence '\.'
go_modules:87:94: W605 invalid escape sequence '\.'
go_modules:112:80: E501 line too long (90 > 79 characters)
go_modules:119:14: F541 f-string is missing placeholders
go_modules:119:80: E501 line too long (82 > 79 characters)
go_modules:138:80: E501 line too long (80 > 79 characters)
go_modules:144:19: F541 f-string is missing placeholders
go_modules:162:80: E501 line too long (85 > 79 characters)
go_modules:172:80: E501 line too long (81 > 79 characters)
go_modules:216:80: E501 line too long (85 > 79 characters)
go_modules:229:80: E501 line too long (83 > 79 characters)
go_modules:254:80: E501 line too long (87 > 79 characters)
go_modules:260:80: E501 line too long (84 > 79 characters)
go_modules:262:80: E501 line too long (89 > 79 characters)
go_modules:271:25: F541 f-string is missing placeholders
go_modules:271:80: E501 line too long (106 > 79 characters)
go_modules:274:25: F541 f-string is missing placeholders
go_modules:274:80: E501 line too long (95 > 79 characters)

I fixed two easy things and added a flake8 GitHub Action:
#28

Not sure if this is desired?
@johanneskastl
Copy link
Author

Fixed some more things, only one error left.

@jfkw
Copy link
Collaborator

jfkw commented Mar 27, 2023

Thanks for the introduction of flake8 CI and the fixes found with it.

I hadn't had a chance yet to compare flake8 formatting results against those of black, apologies for the delay in feedback. @dirkmueller addressed this in d522754. Going forward, we'll need to maintain the flake8 configuration for zero-diff to black output when discrepancies are encountered.

@jfkw jfkw changed the title Python linting? Apply flake8 suggested fixes and add flake8 GitHub CI action with black compatible config Mar 27, 2023
@dirkmueller
Copy link
Member

@jfkw flake8 is just complaining, it isn't reformatting anything.

jfkw added a commit that referenced this issue Mar 27, 2023
@jfkw
flake8 config black compat extend-ignore E501 line too long refs #29
f2dc29c 
Black remains the preferred formatter and allows long lines. Add flake8 config
extend-ignore E501 line too long to supress those warnings.
jfkw added a commit that referenced this issue Mar 27, 2023
@jfkw
Revert wrap of long lines flake8 recommends but black accepts refs #29
7351938 
Default configuration of flake8 recommended wrapping long lines previously
accepted by the black formatter. Revert some of these line changes to reduce
overall diff for security reviewers and to assist automatic rebase of open PRs
currently in review.
jfkw added a commit that referenced this issue Mar 28, 2023
@jfkw
flake8 config black compat extend-ignore E501 line too long refs #29
0506e31 
Black remains the preferred formatter and allows long lines. Add flake8 config
extend-ignore E501 line too long to supress those warnings.
jfkw added a commit that referenced this issue Mar 28, 2023
@jfkw
Revert wrap of long lines flake8 recommends but black accepts refs #29
650f482 
Default configuration of flake8 recommended wrapping long lines previously
accepted by the black formatter. Revert some of these line changes to reduce
overall diff for security reviewers and to assist automatic rebase of open PRs
currently in review.
jfkw added a commit that referenced this issue Mar 28, 2023
@jfkw
Format source with python black refs #29
8c72c30 
Fix incorrect manual revert of flake8 line shortening with the black formatter.

The CI job for the black formatter is configured. Upon PR or push, black CI job
will exit with failure if there is diff to what has been committed.
@jfkw
Copy link
Collaborator

jfkw commented Mar 28, 2023

We can close this issue now. Thanks everyone for their patience with the noisy commits as we configured and experimented with the configuration of the new CI jobs.

@jfkw jfkw closed this as completed Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jfkw @dirkmueller @johanneskastl