search

compilerla / conventional-pre-commit

A pre-commit hook that checks commit messages for Conventional Commits formatting
Apache License 2.0
336 stars 55 forks source link

Mutable reference for tag #10

Closed JMMarchant closed 2 years ago

JMMarchant commented 2 years ago

When running pre-commit autoupdate the rev is first updated from whatever tag to v1 (even from v1.2.0 which is the same commit). Latter autoupdate runs produce the following warning:

[WARNING] The 'rev' field of repo 'https://github.com/compilerla/conventional-pre-commit' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
thekaveman commented 2 years ago

Interesting - I didn't realize that even when you're pinning a specific version tag, pre-commit autoupdate takes you to the mutable tag.

Tried from both v1.1.0 and v1.2.0:

$ pre-commit autoupdate
Updating https://github.com/compilerla/conventional-pre-commit ... updating v1.1.0 -> v1.

$ pre-commit autoupdate
Updating https://github.com/compilerla/conventional-pre-commit ... updating v1.2.0 -> v1.

We use the mutable tag inside VS Code devcontainers, so the latest is built when those containers are rebuilt. But I didn't realize it was causing problems for others using the immutable tags.

@machikoyasuda @angela-tran Given this is not best practice in pre-commit anyway, we should probably rethink our devcontainers workflows to incorporate pre-commit autoupdate instead of using a mutable version tag.

angela-tran commented 2 years ago

Agreed @thekaveman. It sounds like we should get rid of the v1 tag and figure out a way to easily keep our config file up-to-date. I remember us talking about some ideas (around the time that cal-itp/eligibility-server#47 and similar tickets were being worked on).

Some options:

For either option, we should keep in mind hooks that might make (potentially a lot of) changes to files (e.g. flake8 or black): One way to handle that is to use the --repo flag to limit the autoupdate to repos that don't have that potential. That seems a bit avoidant though. Alternatively, we could let autoupdate run on all the hooks, but we will need to also run pre-commit run --all-files so that we see the file changes at the same time as the config file change and review those changes.

thekaveman commented 2 years ago

We're swapping all our usages of the mutable tag for the immutable tag. Once these are merged, we'll delete the v1 tag and maintain the immutable versions going forward.