search

pre-commit-ci / issues

public issues for https://pre-commit.ci
17 stars 4 forks source link

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

Closed alyssadai closed 4 months ago

alyssadai commented 9 months ago

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!

asottile commented 9 months ago

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 commented 9 months ago

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.