-
Notifications
You must be signed in to change notification settings - Fork 27
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
Build Docker images for Ubuntu 22.04 and 24.04 #318
Build Docker images for Ubuntu 22.04 and 24.04 #318
Conversation
Thanks a lot for this. I'm not very familiar with docker, so if someone else might want to step in and review this. Otherwise, I'll just merge it and see what happens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look reasonable but if you wanted to be more idiomatic about it then the Ubuntu version would go in the tag rather than part of the name of the docker image. Also you’d use the code name.
So we’d have $IMAGE_ID:$DB_VERSION-jammy
and $IMAGE_ID:$DB_VERSION-noble
. The latest
tag is usually just aliased to the latest version of the image and the latest version of the most popular base image it uses (so Ubuntu Noble Numbat in this case).
there are naturally many examples of this naming/tagging scheme, but here’s one at random if you want to see it play out at scale on a big project: https://hub.docker.com/_/golang
Thanks Matt, Fortunately ubuntu images allow either |
I might suggest that we also have tags |
run: | | ||
export NO_CACHE="${{ github.ref == 'refs/heads/main' }}" | ||
DB="$DB_VERSION" make docker-build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DB="$DB_VERSION"
didn't appear to be doing anything 🤔
ok I think it's all good now |
Sounds good. As you noted, golang and others do this as well — big projects do tons of aliasing so you can do things like |
indeed, i've just noticed that the tag |
@stefan-hoeck matt's approved, ready to merge when you are |
Thanks again, and thanks @mattpolzin for reviewing this. |
what is this PR? #317