MOD-11297: Revamp the CI pipeline, including publishing - #428
Conversation
4967a89 to
d9165b9
Compare
| @@ -1,11 +1,19 @@ | |||
| [workspace] | |||
| members = [".", "redismodule-rs-macros", "redismodule-rs-macros-internals"] | |||
There was a problem hiding this comment.
is it normal to have this root member like this?
There was a problem hiding this comment.
I generally prefer to have all root members in subfolders, since it makes cargo build in the root of the repository equivalent to cargo build --workspace.
But I know that some projects prefer to have the "key" package at the root level, so no strong preferences here.
| on: | ||
| push: | ||
| branches: | ||
| - master |
There was a problem hiding this comment.
Will this support releasing multiple release integration branches, e.g., a patch for 2.X and a minor for 3.X? (we do not release from master)
Is this a limitation? release-plz/release-plz#2159
There was a problem hiding this comment.
@LukeMathWalker Will this support multiple concurrent releases? ☝🏼
There was a problem hiding this comment.
When looking at the version history, this didn't seem like a requirement—from what I can see, no backports have been made in the last few years.
I would recommend the following setup to minimise complexity:
- Use
masteras trunk and cut releases for the latest major from there (i.e. all2.x.y, until3.0.0is released) - Branch out when a new major release goes out (i.e. create a
v2branch oncemastermoves over to3.z.w)
If we need to backport something, we merge PRs into the backport branch and use something like cargo-release with a short-lived token to cut a release. In the meantime, I can look into adding support for branches to release-plz.
There was a problem hiding this comment.
Once release-plz/release-plz#2438 is merged, I should be able to adjust the workflow to cut backport releases from branches.
|
@LukeMathWalker using #429 #430 I could add myself as owner of redis-module and we could try setting up Trusted Publishing |
2966136 to
699c5d5
Compare
699c5d5 to
10468f8
Compare
629c6a9 to
89c5d76
Compare
89c5d76 to
4cbbd2d
Compare
| @@ -1,2 +1,2 @@ | |||
| #!/usr/bin/env sh | |||
| cargo test --all --all-targets --no-default-features --features min-redis-compatibility-version-7-4 | |||
| cargo test --all --all-targets --no-default-features --features min-redis-compatibility-version-8-0 | |||
There was a problem hiding this comment.
@LukeMathWalker Why are we increasing the min compatibility?
There was a problem hiding this comment.
There were examples referring to min-redis-compatibility-version-8-0, but the feature flag didn't actually exist. I added it and updated test.sh to use it, since it seemed to be designed to test the latest version.
CI still tests all versions (test.sh is never invoked anywhere from what I can see, it's just a local dev helper).
| @@ -33,35 +19,34 @@ jobs: | |||
| - [7-2, "7.2"] | |||
There was a problem hiding this comment.
@LukeMathWalker Should we add 8-0 here? Or unrelated to this PR?
There was a problem hiding this comment.
We could add it now, but since I don't see 7.4 there either, I'm inclined to defer to another PR
For releasing, we switched to
release-plz. It handles version updates in manifests, changelog maintainance and crates.io publishing, thus simplifying the overall workflow.I'd like to avoid using an API token for publishing. We can instead rely on trusted publishing from GitHub Actions. The only prerequisite is a bit of configuration on the crates.io management panel (cc @oshadmi).
I switched the repository to using
stableas the default toolchain. There does seem to be no need fornightlyanywhere.