jorisroovers / gitlint

Linting for your git commit messages
http://jorisroovers.github.io/gitlint
MIT License
791 stars 99 forks source link

v0.19.0dev tag should not be on the default branch #456

Open tueda opened 1 year ago

tueda commented 1 year ago

This is not a bug in gitlint itself, but a problem caused by interaction with pre-commit: running pre-commit autoupdate updates gitlint hook in .pre-commit-config.yaml to v0.19.0dev. This is not good unless v0.19.0dev is more recommended than v0.18.0.

Note that pre-commit document says:

You can update your hooks to the latest version automatically by running pre-commit autoupdate. By default, this will bring the hooks to the latest tag on the default branch.

sigmavirus24 commented 1 year ago

I don't think there's a good fix here. Tags shouldn't be deleted. That commit could be on multiple branches and iirc tags reference the commit, not the branch so "moving" it isn't an option. I don't think pre commit is wrong either because no two ecosystems use the same versioning schemes. This isn't ideal but I don't think there's a fix here

jorisroovers commented 1 year ago

Interesting, pre-commit assumes that tags point to stable versions. (edit: well, it doesn't, the docs clearly state it just points to the latest tag. But as pointed out, users might interpret this incorrectly as pointing to the latest stable version)

I think it’s pretty common for packages to tag pre-release versions. For example, black does this (also available in pre-commit).

In gitlint’s case, this is a recent addition with our adoption of hatch and hatch-vcs in particular, we need this for our dev builds to have versions strings like 0.19.0dev+<sha>

@asottile Perhaps pre-commit autoupdate could support a new flag --stable (or whatever the best name is), so it skips any tags with dev, alpha, beta, rc, a, b in it?

asottile commented 1 year ago

it will not support that

there are many duplicates in the issue tracker

jorisroovers commented 1 year ago

@asottile thanks for responding. I should've done due diligence and searched. Mea culpa.

For reference: https://github.com/pre-commit/pre-commit/issues?q=is%3Aissue+autoupdate+pre-release+is%3Aclosed

Specific issue with a bit more detail: https://github.com/pre-commit/pre-commit/issues/995

sigmavirus24 commented 1 year ago

I would argue that hatch needs to be more flexible here if it isn't already. Honestly non-release tags are a lot of noise, there should be a better way to indicate the next potential version string as a dev string. I'm not familiar with hatch though so, I don't know any options off the top of my head and am on my phone at the moment

jorisroovers commented 1 year ago

So hatch-vcs uses setuptools_scm under the hood.

The Default versioning scheme section in the setuptools_scm README explains how versions are determined.

The part that’s relevant here:

The next version is calculated by adding 1 to the last numeric component of the tag.

Semantic Versioning (SemVer)

Due to the default behavior it's necessary to always include a patch version (the 3 in 1.2.3), or else the automatic guessing will increment the wrong part of the SemVer (e.g. tag 2.0 results in 2.1.devX instead of 2.0.1.devX). So please make sure to tag accordingly.

Since gitlint versioning scheme has a patch release number (i.e. 0.18.0), setuptools_scm by default will bump the following untagged commits after 0.18.0 as 0.18.1devN+g123abc.

This didn’t feel right to me, hence why I added a 0.19.0dev tag, in which case setuptools_scm will recognize the dev suffix and build versions like 0.19.0.devN+g123bc.

I also didn’t entirely like having to pollute the tag list with dev tags, but I decided in the end the trade-off was worth it.

Related side-note: I am actually thinking to do a pre-release for 0.19.0 (and hence add a tag like 0.19.0b1 or 0.19.0rc1) very soon, next week most likely. I want to do a pre-release to be able to test release automation before actually releasing. All the puzzle pieces should be in place for this, I just need to add a trigger to publish to pypi when a new github release is created.

jorisroovers commented 1 year ago

So I noticed that our recent dev builds were being tagged with with 0.19.2devN+g123abc instead of the desired 0.20.0devN+g123abc. I discovered this is because the 0.19.1 tag was added more recently than the 0.20.0dev tag. The way hatch-vcs was configured it didn’t actually parse the semver to determine the latest tag, but just looked at the last added tag to determine the next version.

I was able to fix this by switching the version scheme to release-branch-semver in #478. Now the dev numbers are correctly increased.

A nice side-effect is that we no longer need the dev tags - so I removed them. This probably breaks the git source builds for 0.19.dev versions, but I think that’s an ok trade-off to keep the tag list clean. The release builds will still work fine as those tags remain.

I did still keep the rc tags for 0.19.0 release as well: v0.19.0rc1 and v0.19.0rc2. I think that’s reasonable but understand that keeping those doesn’t solve fully solve this particular issue then. If an rc tag is the latest tag on the repo, it will still be used by pre-commit when auto-updating.

At least an rc should be better than a dev build 🤷‍♂️