-
Notifications
You must be signed in to change notification settings - Fork 148
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 and push docker images using GitHub Actions #249
Conversation
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.
Looking great! Just a couple questions below.
.github/workflows/docker_push.yml
Outdated
# uncomment this to enable all pushes to main | ||
#branches: [ master, main ] |
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.
I think latest
on our docker images always just points to the latest release. Not sure if we'd ever push on the main
branch.
Though you mention re-enabling building docker images on main below - did we used to do that?
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.
I ripped off https://github.com/matrix-org/synapse/blob/develop/.github/workflows/docker.yml which seems like if you push to master on Synapse, you will get a new build.
I'm not really sure why this is, so I took it out for Sygnal.
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.
Given we don't have anything other than releases on our dockerhub, I believe that that workflow both tests docker builds as well as publishes them (if the branch has an associated release tag).
This isn't necessary in Sygnal, as we already have a workflow for testing docker builds.
.github/workflows/docker_push.yml
Outdated
|
||
# The following line can be used to tag builds with the SHA hash of the commit. | ||
# This would be useful if we wanted to re-enable building for all commits on `main`. | ||
# type=sha,enable=true,priority=100,prefix=,suffix=,format=long |
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.
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.
Yeah, that's right. Reckon I should just cut it out entirely?
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.
yeah, I'd cut it.
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
5f1c964
to
256cf9a
Compare
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.
lgtm once the above is sorted.
Now that this is resolved, can we pretty please get a point release tagged and pushed? Looking forward to trying out the db-less sygnal release, but unfortunately the container isn't available on dockerhub. https://hub.docker.com/r/matrixdotorg/sygnal/tags |
That's the plan, yup, sorry for taking my sweet time with it :) |
@bradtgmurray 0.10.1 released -- go and have fun! :D |
Closes #245.