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

Version 3 tag does not point to the correct location #233

Closed
rajbos opened this issue Aug 7, 2023 · 5 comments · Fixed by #237
Closed

Version 3 tag does not point to the correct location #233

rajbos opened this issue Aug 7, 2023 · 5 comments · Fixed by #237

Comments

@rajbos
Copy link

rajbos commented Aug 7, 2023

I noticed discrepancies with the version tag and the branch v3. I assume you where working in the branch v3 to prep for that version, but the tag v3.0.0 points to this Dockerfile @v3.0.0. that has a link to the image @v1, which was not the intent I assume?

What happens now if a user uses this action is confusing:
uses: py-cov-action/python-coverage-comment-action@v3 the branch will be checked out, and Docker image :v3 will be used.
uses: py-cov-action/[email protected] the tag will be checked out, and Docker image :v1 will be used.

I'd recommend cleaning up the branches after merging them to main, and correct the v3 tag to point to the right commit. It's best to keep things straightforward and use tags everywhere, and not branches.

@ewjoachim
Copy link
Member

ewjoachim commented Aug 9, 2023

Versioning of the base image is supposedly independent from the versioning of the action itself. This came because we had no need to change the base image every time we would bump the version. We started having a base image when v3 of the action started. For quite a long time, we stayed on the base image v1. Very recently, we made changes to the base image, twice, so it was bumped to v2 and then v3. By complete chance, the base image version now matches the major version of the action, but it was never meant to be that way and it probably won't stay.

Please note that the base action only has a single component version number (there is no expectation for any kind of backwards compatibility, each commit of the action knows exactly what version of the base image it wants)

Also, please note that the image this action runs on is not exactly the same as the base image. This action runs on the image produced by Dockerfile, whereas the base image is the image produced by Dockerfile.build. The final image is not versionned, we build it (based on the base image) everytime the action runs, and we never push it to Docker Hub. The base image is pushed to Docker Hub

Also, please note that GitHub Action advocates for action version number not to follow the usual practices: v3 means "the latest* incremental version of v3 and not v3.0.0 (as seen here).

So in the end, what we are doing is:

  • v3 is the working branch. It's always pointing to the latest version of the action
  • We sometimes tag, but not very often
  • If someone wants to pin, they should pin on a commit number. Dependabot should still be able to suggest upgrading that commit once in a while.

If we wanted to have everything pristine, we'd need to work on the main branch, and then on each commit to main, force-push v3 tag and v3.x tag to the current commit, and then figure out a new incremental version number v3.x.y and tag that too.

  • This would be exactly identical for users using v3.
  • This would allow users to point to v3.x though it's unclear when we'd make minor versions at all
  • This would allow users to point to v3.x.y but it would be functionally equivalent to pointing to a commit
  • This would have no effect whatsoever on the base image version number

I'm not sure it would be worth the effort, but if you're seeing anything we don't, I'd be delighted to discuss it further :)

I'd recommend cleaning up the branches after merging them to main

We already do that. have you seen anything suggesting the opposite ?

@ewjoachim
Copy link
Member

See #35 and #196 as other similar discussions.

@rajbos
Copy link
Author

rajbos commented Aug 12, 2023

I see the point indeed that v3 is intended as 'latest of whatever is in v3.x.y, and therefor that the newer version of the base image is used in v3. So you decided to update the base image after the release of v3.0.0.

There is also some elegance into have v3 as a branch instead of a tag from the maintainer point of view, as you can continuously work on it and not having to retag the branch where you are working on and overwrite what has been pushed to GitHub. From a users point of view I think it is confusing and that is where my suggestion also came from: a user expects version numbers to be tags, as the version number is expected to come from a release (a release is needed to push the newer version to the marketplace) and a release is linked to a tag. Ofc, v3 is not available on the marketplace either.

I would indeed expect repos to be worked on in the main branch, tag a release (from main) when it is ready and push it to the marketplace from there. That is what I use for my own actions as well. An alpha or beta version tag could come from anywhere.

note: v3.0.0 is also not marked as latest on the marketplace btw, as v2.2.0 was created (and I assume pushed) later. You've must have marked v3.0.0 as the latest release, but that does not propagate to the marketplace it seems.

@ewjoachim
Copy link
Member

note: v3.0.0 is also not marked as latest on the marketplace btw, as v2.2.0 was created (and I assume pushed) later. You've must have marked v3.0.0 as the latest release, but that does not propagate to the marketplace it seems.

Good to know !

@ewjoachim
Copy link
Member

ewjoachim commented Sep 1, 2023

Ok. Now, the default branch is main. Everytime we merge into main, we release v3.<n+1> as a real release (https://github.com/py-cov-action/python-coverage-comment-action/releases/tag/v3.3) and force-push v3 as a normal tag (without a GitHub release).

Also, we have now 2-parts version number (major, minor) and not 3 parts anymore. This means any new release is a minor. Major will only happen if we make a very breaking change, but no such thing is planned for the foreseeable future.

I believe this closes the ticket.

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

Successfully merging a pull request may close this issue.

2 participants