-
Notifications
You must be signed in to change notification settings - Fork 20
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: Add release pipeline #206
Conversation
53cd108
to
3a36afb
Compare
f7d8c5d
to
581aeb1
Compare
NOTES=$(cat << EOF | ||
$BODY | ||
|
||
$EXTENSION | ||
EOF | ||
) |
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.
NOTES=$(cat << EOF
$BODY
$EXTENSION
EOF
)
The spacing threw me off, thought there was a missing parenthesis
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, admittedly this is an esoteric technique for adding the newlines. But it was the only one that worked out for me implementing it. 😄
goarch: arm64 | ||
gpu: false | ||
runner: macos-latest # uses M1 | ||
runs-on: ${{ matrix.runner }} |
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.
Nice! 😍
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.
Are we not doing linux non gpu? Those would be for people running the binary in docker? (assuming most docker images use some flavor of linux)
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.
We do a CPU-only build for every OS and architecture. The gpu
flag indicates whether we should also do a GPU 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.
Thank you for clarifying, I misread the matrix
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.
We take this a step further by automatically merging the release pull request. After we merge, release-please runs a second time to create the release.
😍
Thank you @bgins 🎉
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.
One small Q, but looking forward to having this in place! Great work, @bgins 👏 🚀
.github/workflows/release_docker.yml
Outdated
@@ -2,8 +2,6 @@ name: Publish Docker Image | |||
|
|||
on: | |||
push: | |||
branches: | |||
- "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.
We don't want a latest
docker image that tracks main (for testing, etc)?
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.
Ah yeah, good point. Two questions about it.
Are there changes that are not user facing that should trigger a new Docker image? With the current setup, we will create a new image for every feat:
, fix:
, and breaking change. Are there cases where we would want them for chore:
, build:
, etc?
Second, does ghcr
have a mechanism for listing images by git
tag? They don't list our existing tags ((https://github.com/Lilypad-Tech/lilypad/pkgs/container/resource-provider/versions), but maybe it's because of the way we have been formatting them. The concern here is whether or not resource providers can find the latest released version vs the latest published version.
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.
Are there changes that are not user facing that should trigger a new Docker image? With the current setup, we will create a new image for every
feat:
,fix:
, and breaking change. Are there cases where we would want them forchore:
,build:
, etc?
Maybe not? I think I don't know the answer yet.
Second, does
ghcr
have a mechanism for listing images bygit
tag? They don't list our existing tags ((https://github.com/Lilypad-Tech/lilypad/pkgs/container/resource-provider/versions), but maybe it's because of the way we have been formatting them. The concern here is whether or not resource providers can find the latest released version vs the latest published version.
It will publish on git tags - I think we just haven't done one since the docker build step was added.
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.
Alright yeah, if the tagged versions will be distinct on ghcr
, let's publish images on push to main
and tags. Will drop the change.
Can imagine a scenario where we are updating the Dockerfile, it's a build:
task not a fix/feature, and we want the team to have easy access to an image.
581aeb1
to
a9eac7c
Compare
a9eac7c
to
26e6d05
Compare
Review Type Requested (choose one):
Summary
This pull request adds a release workflow that:
It also makes a few associated changes:
testnet_deploy_services.yml
to deploy only on releasesUpdate. Removed. We also want to publish on push torelease_docker.yml
to publish only on releasesmain
.-gpu
suffix in release name (and rename script tolilypad-updater-gpu.sh
)Task/Issue reference
Closes: #91, closes #168
Details
Goals
Our goals for the release pipeline:
main
release-please
This PR uses
release-please
to generate release pull requests and create releases when new features or fixes are introduced.release-please
checks for conventional commit prefixes to determine a semantic version and to decide when to create a release. From their documentation:release-please
does not create release PRs or releases for tasks without user facing (prefixes likechore:
,build:
, etc).Automate it more please
A typical
release-please
setup creates a release pull request that a human approves and merges. Once the PR is merged,release-please
runs a second time and creates a release when it observes its release commit.We take this a step further by automatically merging the release pull request. After we merge,
release-please
runs a second time to create the release.Here is a diagram showing an example flow on commits coming into
main
and how the releases workflow responds to each. The diagram assumes we have started at version1.0.0
:We have created a
lilypad-releases
bot (implemented as a GitHub app) to runrelease-please
actions. The custom bot is needed to trigger further actions, such as running the workflow a second time on merge.In addition, our branch protection rules for
main
require one PR approval and will require passing tests in the near future. Thelilypad-releases
bot cannot approve it's own pull request, so we have the defaultgithub-actions
bot step in for approval.Customization
release-please
automatically maintains a CHANGELOG, generates pull request text, and generates release notes.We can customize the pull request text by updating release-please-config.json.
We extend the generated release notes with an
extend-notes
job. This job reads arelease_notes.md
file and appends it to the generated release notes. We can customize the release notes by updating this file, but note that the changes will persist in the next release unless they are changed again.How to test this code?
This pull request is challenging to test in this repository because we need to merge it to see it in action.
I have created a Lilypad-Tech/test-gh-action-releases repo to demonstrate the release workflow with a few test PRs ready to approve and merge: https://github.com/Lilypad-Tech/test-gh-action-releases/pulls
Anything else?
Our internal version to capture the design can be found here: https://www.notion.so/lilypadnetwork/Release-Pipeline-70ecc09bed8b406ba33e7e1e4c4e93b3