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

meta: Remove black GH action #1339

Merged
merged 2 commits into from
Feb 14, 2022
Merged

meta: Remove black GH action #1339

merged 2 commits into from
Feb 14, 2022

Conversation

sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Feb 14, 2022

Since we moved to pre-commit in #1315 anyway, we don't need this flaky black stuff to run on push.
The lint step checks with black --check either way.

@BYK
Copy link
Contributor

BYK commented Feb 14, 2022

@sl0thentr0py you still need someone with repo admin access to remove that required format check in branch protection rules.

@antonpirker
Copy link
Member

I am also for removing it.

One thing we might keep in mind: people submitting PRs maybe do not have pre-commit installed so can submit horribly formatted code.

@sl0thentr0py
Copy link
Member Author

sl0thentr0py commented Feb 14, 2022

I wanted to bring this up in the pre-commit PR but anyway, let's do it now.
We already have make format here, now that we added pre-commit, that's redundant.
I initially thought why add a new tool when we can just run make format in a simple git pre-commit hook?
But anyway, now that we added it, we can just change the Makefile and make users setup pre-commit in CONTRIBUTING.md.

@BYK
Copy link
Contributor

BYK commented Feb 14, 2022

One thing we might keep in mind: people submitting PRs maybe do not have pre-commit installed so can submit horribly formatted code.

@antonpirker I tried that, your linter yells at me so nothing to worry about 🤣

@sl0thentr0py sl0thentr0py enabled auto-merge (squash) February 14, 2022 18:09
@sl0thentr0py sl0thentr0py merged commit 6649e22 into master Feb 14, 2022
@sl0thentr0py sl0thentr0py deleted the remove-black branch February 14, 2022 18:36
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.

None yet

3 participants