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

Enforce Python code standards with black and flake8 #490

Merged
merged 5 commits into from
Nov 17, 2021

Conversation

shanemcd
Copy link
Member

Code standards are important, but I hate spending any amount of time actually thinking about it. So, let's automate that activity out of our lives.

@shanemcd shanemcd merged commit 934780f into ansible:devel Nov 17, 2021
@thenets
Copy link
Member

thenets commented Nov 17, 2021

@shanemcd I believe that we need to talk about this before enforcing a code style. The black library is not even configured to follow the tox asserts.

For Go it's easy because the go fmt does the job. For Python, not so much. Libraries like black and tox do not follow the same defaults.

I decided to disable at least the receptorctl-lint until we fix these issues and decide on one specific code style here:

@shanemcd
Copy link
Member Author

@thenets We already use black in AWX, and we had a team discussion around its use when it was adopted. I'm not sure what you mean by "not even configured to follow the tox asserts."

@thenets
Copy link
Member

thenets commented Nov 17, 2021

Already solved while talking to @shanemcd, but to give context to other people:

My mistake about the libraries' names. The issue was about black and flake8.
The current problem is about the line length. By default, black doesn't follow the same size as flake8, but even passing this size as a param, the comments don't have any formating process, which will fail during the lint step.

The current solution needs some manual changes in some cases. We can search later about a better way to run black automatically.

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