`no-commit-to-branch`: allow committing to `main`/`master`

[doublethink13]

We should be able to commit to main/master even if we are using this hook. This is especially helpful in pipelines.

I run pre-commit in my CI/CD because I want all the checks to be run there in case a developer does not have the pre-commit hooks working properly in their local development, so skipping this hook is a problem to me.

After reading the source code, I think we can:

• Update this comparison by removing or ('master', 'main')
• Add a flag called allow-commits-to-main that is True by default and use it somehow in here as well

If this is a good idea, I'd like to take a shot at it 🙂

[asottile]

please search the issue tracker, there are established solutions

[doublethink13]

Would you be kind enough to point where exactly in the issue tracker there is an explanation regarding how to use no-commit-to-branch while being able to commit to main/master?

I understand we can use SKIP=no-commit-to-branch, but then we wouldn't be using this hook after all.

There are valid use cases to commit to main/master (one might go so far as to say that we are all required to commit to main/master, especially if that's the name of our default/production branch), so this tool should allow that by default or via config option at least.

[asottile]

SKIP=no-commit-to-branch is the solution, I'm not going to add additional complexity beyond that since it already solves the problem nicely

[doublethink13]

How come you consider a good solution to NOT run the solution?

I'll say it again, committing to main/master is a perfectly valid workflow. We need to merge to main/master when merging PRs. Hotfixes are a thing. I'm sure if we give it a good thought we would come up with many more use cases.

I can even make the case for being impossible to commit to main/master if this hook is present without a workaround to add more complexity to the tool than the proposed solutions. I certainly didn't expect it to happen, and time will tell if someone else will agree with me.

Also, even if you are removing complexity from the tool, you are certainly adding complexity to a developer's workflow by having to accommodate workarounds to very simple and valid use cases.

Sensible defaults are a thing for a reason. Even if you or the community don't think committing to main/master is a sensible default, having the option to commit to main/master should be a sensible default.

Anyway, I think I said my piece and I can understand where you are coming from. I'll consider the issue closed if you won't change your mind. Thanks for your time and the prompt replies 🙂

[asottile]

you do also realize that main / master is just the sensible default and that if you configure --branch it will not use that sensible default

can you explain to me why you don't think SKIP is a solution?

I'm not sure where you're lost so I don't know how to help you

[doublethink13]

I'm not sure where you're lost so I don't know how to help you

If I'm on the main branch, and I have this .pre-commit-config.yaml:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: no-commit-to-branch

I would expect to be able to commit to the main branch, and, in fact, any branch. Alternatively, it could be good UX to have an error if we don't configure any branch to prevent commits to.

I also tried the following .pre-commit-config.yaml, but all of them prevent me to commit to main:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: no-commit-to-branch
        args:
          - --branch
          - main
      - id: no-commit-to-branch
        args:
          - --pattern
          - main
      - id: no-commit-to-branch
        args:
          - --pattern
          - "!main"
      - id: no-commit-to-branch
        args:
          - --pattern
          - ^(?!main$).*
      - id: no-commit-to-branch
        args:
          - --pattern
          - ^main$

As a user, I would expect one or more hooks in the following .pre-commit-config.yaml to allow me to commit to the main branch:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: no-commit-to-branch
      - id: no-commit-to-branch
        args:
          - --allow
          - main
      - id: no-commit-to-branch
        args:
          - --allow-commits-on-main
      - id: no-commit-to-branch
        args:
          - --allow-commits-on-default

Besides SKIP, is there any way of configuring this hook to allow commits to main?

can you explain to me why you don't think SKIP is a solution?

The reason why my teams uses pre-commit is to enforce a set of hooks that prevent unwanted actions from being committed.

Ideally, every developer has pre-commit correctly configured and enabled in their local development environment.

However, errors and mistakes happen, so we run, in the CI/CD, all the hooks that should have been run in the local development environment.

We are using GitHub Actions. Right now, this is the workaround we have to allow us to run all the other hooks on main, and all hooks otherwise:

# ...
      - if: ${{ github.ref_name != 'main' && github.ref_name != 'master' }}
        name: pre-commit hooks
        uses: pre-commit/action@v3.0.1
        with:
          extra_args: --all-files --verbose
      - env:
          SKIP: no-commit-to-branch
        if: ${{ github.ref_name == 'main' || github.ref_name == 'master' }}
        name: pre-commit hooks
        uses: pre-commit/action@v3.0.1
        with:
          extra_args: --all-files --verbose
# ...

I think we should not need to work around pre-commit. Allowing to commit to main seems something that should be achievable using only the hook in question.

Finally, the reason why setting the env var SKIP is not a good solution is two fold:

• If doing A results in B, and we need B, the solution cannot be to not do A, because, by definition, we won't have B. In this case, B is not having commits to certain branches. So the solution cannot be to not run no-commits-to-branch, because we won't be sure we won't have commits to certain branches
• A developer could run export SKIP=no-commit-to-branch && git commit -m 'test'. But then what would the point be of having a no-commit-to-branch if a developer can skip things in their local development environment? What's the point of having pre-commit if anyone can skip it locally and/or in the CI/CD?

[asottile]

ok I think I understand your core misunderstanding

I would expect to be able to commit to the main branch, and, in fact, any branch

the whole point of no-commit-to-branch is to prevent an accidental commit locally to your protected (by default main) branches

[asottile]

But then what would the point be of having a no-commit-to-branch if a developer can skip things in their local development environment?

ah you have another fundamental misunderstanding -- client commit hooks can always be bypassed (pre-commit provides some conveniences with SKIP but there's also the --no-verify sledge hammer) -- they are not a gating mechanism

[asottile]

what branches are you trying to prevent commits to in the first place?

[doublethink13]

the whole point of no-commit-to-branch is to prevent an accidental commit locally to your protected (by default main) branches

I don't think the documentation is sufficiently clear on this. When something is something by default, it implies it can be some other thing If the default is main and master, then by configuration it can be some other name and not main or master. That's not the case. main/master are always protected.

I would understand if this .pre-commit-config.yaml would protect EVERY branch:

repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: no-commit-to-branch

But that's not the case. This .pre-commit-config.yaml allows me to commit to testbranch but not to main.

And since I can protect/unprotect any other branch, I would expect this to be true for the main and master branches as well. But it's not. They are always protected.

ah you have another fundamental misunderstanding -- client commit hooks can always be bypassed (pre-commit provides some conveniences with SKIP but there's also the --no-verify sledge hammer) -- they are not a gating mechanism

While it's true a developer can commit using --no-verify, if we manually run pre-commit in the CD/CD, mistakes can be caught and corrected. SKIP is dangerous because of this.

what branches are you trying to prevent commits to in the first place?

Because of requirements, besides having and committing to main, we need all our branches to be named something like feature/TOPIC-123/explanation.

Except for the main branch, I can accomplish this just fine 🙂

[asottile]

I don't see how you can interpret this any other way than what it does:

• Use args: [--branch, staging, --branch, main] to set the branch. Both main and master are protected by default if no branch argument is set.

if you don't want main and master protected you need to specify --branch in some way shape or form

if we manually run pre-commit in the CD/CD

"manually" and "C"I are mutually exclusive -- no-commit-to-branch makes exactly zero sense in CI because it is not a human at a computer that might commit to the incorrect branch

I have to assume you're trolling at this point so I'm going to lock this