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

pre-commit-ci bot #1460

Open
d-v-b opened this issue Jul 12, 2023 · 4 comments
Open

pre-commit-ci bot #1460

d-v-b opened this issue Jul 12, 2023 · 4 comments

Comments

@d-v-b
Copy link
Contributor

d-v-b commented Jul 12, 2023

over in #1459 I'm trying to change the pre-commit configuration without styling the code, because I want that to be a separate PR, as per the suggestion here. The logic of that suggestion was to separate the styling configuration changes (which should be attributed to me in git blame) from the styling itself, which will touch the entire codebase and probably not be attributed to anyone in git blame.

However, this project now has a bot that automatically applies the pre-commit hooks to pull requests, even for files that haven't been committed. See f1d1372 Because of this bot, it's impossible to do separate PRs for this styling change.

What is the argument for using this bot? I feel like the styling / linting stuff should be part of the checks PRs must pass rather than changes that are automatically applied to PRs.

@Saransh-cpp
Copy link
Contributor

You should be able to use [pre-commit.ci skip] in your commit message to stop the bot from acting on a commit.

@d-v-b
Copy link
Contributor Author

d-v-b commented Jul 12, 2023

You should be able to use [pre-commit.ci skip] in your commit message to stop the bot from acting on a commit.

That's good to know! It looks like this is documented in more detail here: https://pre-commit.ci/

However, I'm still not sure why this bot exists, rather than making linting part of the checks for a PR

@rabernat
Copy link
Contributor

rabernat commented Jul 13, 2023

I am not a fan of having the bot make commits. I feel like this creates friction in the developer workflow. I'd be in favor of changing the configuration. Thoughts @joshmoore?

@joshmoore
Copy link
Member

joshmoore commented Jul 13, 2023

I don't have a strong feeling either way, i.e. there are pros and cons of both. I think this was mostly because the kind folks that were working on linting during gsoc & outreachy were in favor, but I agree that this should most importantly work for active maintainers. As long as it gets fixed per PR rather than breaking things deeper in the release process, I'll be happy.

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

No branches or pull requests

4 participants