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

autoupdate makes formatting changes in update PRs when autofix_PRs: false #207

Open
alyssadai opened this issue Feb 28, 2024 · 2 comments
Open

Comments

@alyssadai
Copy link

alyssadai commented Feb 28, 2024

Hi @asottile,

Thanks a lot for your work developing pre-commit/pre-commit-ci.

I was wondering if there is any way to configure the autoupdating function of pre-commit-ci to also respect the autofix_PRs: false setting, such that if an autoupdate includes a hook bump that introduces new formatting rules, that formatting is not applied automatically as part of the PR which updates the hook. We experienced this with the black update recently, see: https://github.com/neurobagel/api/pull/278/files for example (we do not want the autoupdate to touch non pre-commit-config.yaml files automatically).

Our pre-commit-ci config:

ci:
  autofix_prs: false
  skip: [docker-compose-check]
...

Appreciate your help!

@alyssadai alyssadai changed the title autoupdate makes formatting changes in update PRs when autofix_PRs: False autoupdate makes formatting changes in update PRs when autofix_PRs: false Feb 28, 2024
@asottile
Copy link
Member

the auto update prs will always include the auto fixes if available

is there an actual reason you don't want this to happen? the setting is only intended for curmudgeony-avoiding-needing-pulls for user feature branches and really is not recommended at all

@alyssadai
Copy link
Author

alyssadai commented Feb 28, 2024

Mainly for consistency with the behavior on the rest of our non-autoupdate PRs, which respect that setting. Generally, we've set autofix: False to avoid merge conflicts for multiple contributors working on the same PR. While this should be less of an issue for autoupdate PRs alone, if the auto-reformatted code includes sections being worked on in other open PRs (which are not autofixed by default), I wonder if this could lead to confusing conflicts.

If there is a strong motivation to keep the autoupdate+autofix coupled regardless of autofix value in the config, would it be possible for the ci docs for this option to be updated to reflect this behaviour?

Happy to open a PR for the docs if that is preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants