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

The publish action should increment the minor version component #16

Open
iaindillingham opened this issue May 8, 2024 · 6 comments
Open

Comments

@iaindillingham
Copy link
Member

iaindillingham commented May 8, 2024

At present, the publish action tags a new image as "v0", meaning that "v0" can resolve to different images at different times.

- name: Publish image
run: |
echo ${{ secrets.GITHUB_TOKEN }} | docker login $REGISTRY -u ${{ github.actor }} --password-stdin
docker tag $IMAGE_NAME $PUBLIC_IMAGE_NAME:v0
docker tag $IMAGE_NAME $PUBLIC_IMAGE_NAME:latest
docker push $PUBLIC_IMAGE_NAME:v0
docker push $PUBLIC_IMAGE_NAME:latest

Semver considers using major version zero for initial development to be optional:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

However, it also considers modifying the package's contents after the package is released to be an absolute prohibition:

Once a versioned package has been released, the contents of that version MUST NOT be modified. Any modifications MUST be released as a new version.

So, whilst we are doing the right thing by using major version zero 🙂, we are doing the wrong thing by repeatedly tagging new images as "v0" 🙁. Instead, we should be incrementing the minor version component.

We took a similar approach when developing ehrQL. A user specified "v0" in their project.yaml, with the minor version component resolving to the most recent available release in the execution context (the local context tended to lag the secure backend context). The inconvenience of a backwards-incompatible change was deferred by specifying the minor version component. The user was expected to refactor at a more convenient time, as the most recent available release in the secure backend context really was the most recent available release overall.

@lucyb
Copy link
Contributor

lucyb commented May 20, 2024

At present, the publish action -- which is run on dispatch, on PR, and on push to main

Oh no, I wasn't expecting that to be the case. I was intending for the publish action to only run when something was merged to main. I thought this line would do that:

if: github.ref == 'refs/heads/main'

Do we need to push any images if they're on a branch? I can see how that might be useful, but it adds quite a bit of complexity. Maybe it would be better to fix the publish action, so that it does what I originally intended instead.

@iaindillingham
Copy link
Member Author

Sorry for the confusion, @lucyb. You are correct! I will remove "which is run on dispatch, on PR, and on push to main" from the description. However, the issue I'd intended to raise is that one user's "v0" won't necessarily be another user's "v0" because dev containers can live for 14 days' when run in Codespaces and indefinitely when run locally.

This is undesirable from our perspective because it breaks semver. In other words, this is undesirable because it violates the principle of least surprise. This is also undesirable from a user's perspective, because the user can't pin the version of a dev container. (We should recognise that whilst it makes our life easier if they don't do so, it won't make their life easier.)

@lucyb
Copy link
Contributor

lucyb commented May 23, 2024

I'm taking this off the board while we discuss it.

we are doing the wrong thing by repeatedly tagging new images as "v0" 🙁. Instead, we should be incrementing the minor version component.

I disagree. Tagging new images as "v0" shows they are the latest version of that major version. This is desirable because users will get the latest version without having to make any changes.

However, I do think we need to tag images with the minor version as well. This would match up with the versioning proposal document. This would allow users to pin to an unchanging minor version if they wanted.

Is part of the problem that we're using "v0"? Should we actually be moving to v1 at this point?

dev containers can live for 14 days' when run in Codespaces

Just to clarify, a Codespace can live for longer than 14 days. They are automatically deleted when they've been inactive for 14 days. I'm unsure what changes are pulled in when a Codespace is restarted though, which will happen more frequently (at least daily, I imagine).

@iaindillingham
Copy link
Member Author

Thanks, @lucyb.

I think that we've been circling around the implementation of a version and the concept of a version. I agree that we should tag each image with its major and minor components. For example, by executing both docker tag ...:v0 and docker tag ...:v0.1. If we do, then a user can access the version of the image that's important to them. Here, "tagging" is how we implement the version. Conceptually, however, we should identify an image by its major and minor components. At present "v0" could mean "v0.1", "v0.2", or "v0.k"

I do not think we should be using "v1" at present. "v0" gives us flexibility. If we move to "v1", then we need to be very careful not to break backwards compatibility. I don't think our tests are comprehensive enough at present.

@StevenMaude
Copy link
Contributor

One consideration here I just noticed. As things stand, as I understand it, any merge to main results in the publication of a new image, regardless of whether anything changed in the image or not.

It might be that the image hasn't actually changed since the last publication; for example, editing the tests, which would give an incremented vX.Y.Z version number that's potentially confusing when nothing has changed in the image.

@iaindillingham
Copy link
Member Author

I don't think there's anything wrong with incrementing the patch version under that scenario. It means you don't have to remember to use a conventional commit, right after you forget to use a conventional commit.

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

No branches or pull requests

3 participants