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 pre-commit.ci badge #2148

Merged
merged 3 commits into from
Mar 12, 2023
Merged

Conversation

bjlittle
Copy link
Member

This PR adds the pre-commit.ci badge to the README.md as a follow on to #1934

Controversially, I've also removed the autofix_prs disable in the .pre-commit-config.yaml - totally happy to encourage debate on this choice.

Personally, the pre-commit.ci autofix does save the exact situation that we're initially in, where we banked a PR that wasn't compliant for all the git-hooks (#2146 - my bad, I should have rebased to include #1934 and the git hooks would have detected non-compliance locally).

Naively, I trust the hooks to do the "right thing" and auto-fix as part of the pre-commit.ci workflow. I'd love to hear the flip argument as to why we shouldn't adopt that pattern.

@bjlittle
Copy link
Member Author

bjlittle commented Mar 12, 2023

@greglucas Can we please, please, please disable stickler-ci 🙏

I'd be soooo happy to do that 😄

I'd argue that our goal now is to fully adopt the pre-commit.ci workflow, given #1934 has landed i.e., put all our eggs in the pre-commit git-hook basket, and therefore we can purge the unnecessary use of the stickler-ci service. It's served its purpose - time to retire it, I say 😉

Thoughts?

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@bjlittle
Copy link
Member Author

pre-commit.ci autofix

@bjlittle bjlittle mentioned this pull request Mar 12, 2023
@greglucas greglucas merged commit f20c935 into SciTools:main Mar 12, 2023
@greglucas greglucas added this to the 0.22 milestone Mar 12, 2023
@bjlittle bjlittle deleted the add-pre-commit-ci-badge branch March 12, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants