-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add release workflow #19
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
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.
As discussed, I don't think this is the best approach. It forces the person who will run the process to install tooling, read the process documentation; running this version of the process means jumping between browser and CLI.
The same process could be more simple from the operator perspective, for example:
- manually run workflow that: opens PR with suggested version and changelog changes
- once PR is approved and/or merged, another automatically triggered workflow kicks in, which builds the binaries and uploads them to GH Releases.
Doing it ^that way ensures the easiest time for the process operator (no knowledge about/installation of tooling is necessary, and the process documentation can boil down to a single sentence), which I think should be the ultimate goal for repetitive, frequently occurring processes that don't change often, which is the case here.
Nevertheless, I'm approving, because you said you don't wish to be blocked.
- name: Install dependencies | ||
run: | | ||
wget https://github.com/cargo-bins/cargo-binstall/releases/latest/download/cargo-binstall-x86_64-unknown-linux-musl.tgz | ||
tar -xvzf cargo-binstall-x86_64-unknown-linux-musl.tgz | ||
rm cargo-binstall-x86_64-unknown-linux-musl.tgz | ||
mv cargo-binstall $HOME/.cargo/bin | ||
cargo binstall cargo-release -y |
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.
- name: Install dependencies | |
run: | | |
wget https://github.com/cargo-bins/cargo-binstall/releases/latest/download/cargo-binstall-x86_64-unknown-linux-musl.tgz | |
tar -xvzf cargo-binstall-x86_64-unknown-linux-musl.tgz | |
rm cargo-binstall-x86_64-unknown-linux-musl.tgz | |
mv cargo-binstall $HOME/.cargo/bin | |
cargo binstall cargo-release -y | |
- uses: cargo-bins/cargo-binstall@main | |
- name: Install dependencies | |
run: cargo binstall cargo-release -y |
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.
Thanks! We have some technical debt in another repo, then: https://github.com/dfinity/cycles-ledger/blob/main/.github/workflows/release-with-github.yml#L43
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.
thanks, will take care of it. I made cycles-ledger workflow before I contributed uses: cargo-bins/cargo-binstall@main
to their repo
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.
- name: Roll changelog, bump version, and push branch | ||
run: | | ||
# see https://opensource.axo.dev/cargo-dist/book/workspaces/cargo-release-guide.html#using-cargo-release-with-pull-requests | ||
cargo release "${{ inputs.level }}" --execute --no-confirm --config prepare-release.toml |
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.
Is there a benefit of having a separate .toml file for release config over storing the configuration in root Cargo.toml
?
I also think that release-prepare.toml
(vs prepare-release.toml
) is easier to find and mentally parse when eyeballing files in the repo, and conveys same msg
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.
The benefit is that cargo release
doesn't roll the changelog
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 don't understand. The --config
points to a file that has changelog rolling configured.
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.
Unless you mean calling cargo release --execute
as last step of the process (as mentioned in contributing.md) has a different behavior. If that's the case, then I find it confusing or at least, not very obvious
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.
When the GH workflow runs cargo release ... --config prepare-release.yml
, it needs to roll the changelog, so that that goes in the PR.
After the PR is merged, the cargo release --execute
final step should not roll the changelog (which would create an additional commit).
echo "PR created by this workflow: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" >> BODY.md | ||
echo "After merging, run the following:" >> BODY.md | ||
echo "'''bash" >> BODY.md | ||
echo "git checkout main" >> BODY.md | ||
echo "git pull" >> BODY.md | ||
echo "cargo dist plan" >> BODY.md | ||
echo "cargo release --execute" >> BODY.md | ||
echo "'''" >> BODY.md |
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.
using heredoc would improve readability
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.
Thanks! I didn't know that was possible in a yaml file.
prepare-release: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 |
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.
- uses: actions/checkout@v3 | |
- uses: actions/checkout@v4 |
|
||
- name: Install Rust | ||
uses: dtolnay/rust-toolchain@stable |
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.
- name: Install Rust | |
uses: dtolnay/rust-toolchain@stable |
- rust comes preinstalled on ubuntu runner
dtolnay/rust-toolchain
won't install rust version listed inrust-toolchain.toml
- if you wish to enforce the version of rust listed in
rust-toolchain.toml
you must callrustup show
(until this is solved Command for "install stuff fromrust-toolchain.toml
" rust-lang/rustup#2686 (comment))
Description
Adds the release process workflow and documentation.
Notes:
cargo release --execute
step does not also roll themcargo release
looks for configuration is$WORKSPACE/release.toml
, so this sibling seems reasonable.Fixes https://dfinity.atlassian.net/browse/SDK-1277
How Has This Been Tested?
I wasn't able to fork this repo (I imagine because it is private), so I made a different repo and then copied the changes here.
I had to open the PRs manually, because
gh pr create
failed. It's possible that some repo setting is required, or that this is enabled at the org level.