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 black.yml format check github action, and reformat remaining files in repo that did not pass check #668

Closed
wants to merge 2 commits into from

Conversation

jdegenstein
Copy link
Collaborator

This will e.g. check new PR for black formatting and fail if it was not properly formatted. I chose to use ~= 24.0 as the version for now.

@gumyr
Copy link
Owner

gumyr commented Aug 20, 2024

From what I understand projects put a requirement on using a white space enforcement tool to reduce the effort in accepting PRs as white space changes aren't flagged and therefore not reviewed. However, so far I haven't found this to be a problem and I worry it may reduce PRs as there is yet another requirement to be met. As an example of what might happen, there are a few long standing open PRs that require unit tests where the authors aren't willing to add them.

I'm happy to have the files periodically cleaned up with black though. Would this be an acceptable compromise?

@jdegenstein
Copy link
Collaborator Author

Yes, I don't feel this is a critical issue to solve so I am OK with your compromise.

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