ci: release process document #2153
Conversation
jdspdx
force-pushed
the
jared/release_process
branch
from
43e968b
to
1ede032
Compare
jdspdx
force-pushed
the
jared/release_process
branch
from
1ede032
to
e49e661
Compare
There was a problem hiding this comment.
Thanks for the thorough review. Looking at the current crate-publisher.sh we should be able to further reduce commands.
As suggested, we can reduce the script to only update the readme and cargo version and deps, since those are manually done currently.
We should also further investigate how we can use cargo release workspace feature to fully automate our release process and also integrate it with our Changelog generation.
With the ability to exclude crates we don't want to release, automatic crate publish order and also incoming feature to exclude crates that don't have changes comparing it to tags, we should be able to create a leaner script that does the following with few commands.
- Generate Changelog with
git commit - Call
cargo releaseto update version, readme, and dependencies of all crates.
| - For each changed crate: | ||
| - Run `cargo release minor --skip-tag --skip-push --skip-publish --no-dev-version --execute` | ||
| - Run `git reset` to `HEAD` | ||
| - Run `git add --all` | ||
| - Run `git commit -m "committed all crates"` |
There was a problem hiding this comment.
We are not using git commands for bumps
https://github.com/ockam-network/ockam/blob/30f44e51017b8e3d10a0fba7b49c7fc606ed58fa/tools/scripts/release/crate-publisher.sh#L108
- ^
cargo releaseis called for all crates so as to bump their versions and also dependencies.
- ^ If a
cargo releasecommand fails for a specific crate, the script thenresetsall changes that were made and pauses so that we can debug the failing crate that is to be released and then continue with deployment or abort.
| - Run `git reset HEAD` again | ||
| - Prints "cargo release commits squashed" ? | ||
| - Pause and prompt user to verify `git diff` | ||
| - Run `git add --all` again | ||
| - Run `git commit` again |
There was a problem hiding this comment.
This can be improved, my initial assumption was that cargo release doesn't allow us exclude crates over cli, apparently I was wrong. I believe we can call cargo release with the workspace feature link here (exclude field), we can get crates that are to be excluded using our Changelog script and then indicate to cargo release crates that should not be released.
| - Pause and prompt user to verify `git diff` | ||
| - Run `git add --all` again | ||
| - Run `git commit` again | ||
| - FORCE PUSH to current branch: `git push --set-upstream origin "$current_branch" -f` |
There was a problem hiding this comment.
This is more of
- Will the user create a new branch which this script will be called on?
- Will the branch be published upstream?
- Will there be conflicts on the branch?
| - The critical steps that this process needs to accomplish are: | ||
| - Version increment crate `Cargo.toml` for changed crates | ||
| - Find and replace new version in `README`s | ||
| - Update interdependencies in crates' `Cargo.toml` | ||
|
|
||
| These are all simple text manipulation tasks. None of this should require so many git calls. |
There was a problem hiding this comment.
https://github.com/ockam-network/ockam/blob/30f44e51017b8e3d10a0fba7b49c7fc606ed58fa/tools/scripts/release/crate-publisher.sh#L108
^ We use cargo release for the 3 operations, we are not performing git calls till only if the bump fails
| ### Using diff from main instead of finding last tags | ||
|
|
||
| - This process relies on the fact that main and develop diverge between releases, and compares them to find the differences. | ||
| - It is more accurate to find the commit hash of the last crate tagging. Since currently all tags happen at the same time, this is a single commit. | ||
| - If for some reason `main` diverges for non-release reason, such as maintenance or bug fixes, this could cause problems |
There was a problem hiding this comment.
Love this. I totally agree with this, I believe we can make the current script better and leaner.
| ### Using the Changelog version as the source for version info | ||
|
|
||
| We should not rely on Changelog for the accuracy of the crate version. The crate version should come from `Cargo.toml`, where we _know_ it is accurate. If the version is wrong in `Cargo.toml`, it is apparent and things will break. If the version is wrong in the Changelog, the script will just use it and believe it. | ||
|
|
||
| The Changelog is metadata for humans, tooling should not rely on it. |
There was a problem hiding this comment.
💯 we shouldn't use the Changelog as the source of truth.
| ### Static list of crates | ||
|
|
||
| - This release process requires a static list of crates embedded in the script, which has already become outdated. | ||
| - Should not be necessary: we know exactly what our list of crates is from our repo directory structure. | ||
| - Instead we should have a way to omit crates that we do not want to publish (if any) for example the `ockam_examples` | ||
| - Further more, this static list must be ordered by internal dependencies, and is currently incorrect. | ||
| - Internal dependencies can and do change often. This is hard to maintain. |
There was a problem hiding this comment.
Thanks Jared. This was a concern for me we should be able to solve this using cargo release.
| ## General script concerns | ||
|
|
||
| - Relative directories are used too much. See the existing scripts strategies of utilizing an `$OCKAM_HOME` variable, which can be used as the base to define other locations. | ||
| - Use `pushd` and `popd` to manage directory state instead of `cd` | ||
| - Remove all `$(tput setaf 2)` and `$(tput sgr0)` from message lines. They add too much noise and obscure what is going on. | ||
| - Consider making a function named something like `print_message` which injects these colors, if necessary. | ||
| - The presence of `gh` is assumed but not checked |
| # Suggestions | ||
| - Split the script into two scripts: | ||
| - One script to prepare the release branch | ||
| - One script to publish to GitHub and crates.io |
There was a problem hiding this comment.
Thanks. This is something I'll look into critically. We should also further look into how we can use the workspace feature in cargo release. There's also a PR that includes the --exclude-unchanged arg that should detect crates that don't need to be bumped so we can release with few commands.
| ## Release branch script | ||
| - Remove all git operations other than those used to build Changelogs. | ||
| - Remove ordered list of crates and use the directory structure of the repo to determine crates. For this script, dependency order is not important. | ||
| - Use existing `crate-changes.sh` script to automatically populate Changelogs | ||
| - Increment version numbers | ||
| - Update READMEs and dependant `Cargo.toml`. | ||
| - The end result should be changes staged to a release branch that includes all version increment work. |
There was a problem hiding this comment.
👍 Thanks Jared. We can start with this.
Creates RELEASE.md which documents our existing release process, proposed new process, and existing concerns.