[draft] split publishing crates in two steps, core crates then cli#2256
[draft] split publishing crates in two steps, core crates then cli#2256victornicolet wants to merge 6 commits intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Victor Nicolet <victornl@amazon.com>
Signed-off-by: Victor Nicolet <victornl@amazon.com>
victornicolet
force-pushed
the
victornicolet/release-workflow-split-cli-and-core
branch
from
fa31d1b to
ce4fe29
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the release automation by moving repeated publish/validation logic into a reusable GitHub Actions workflow, and splitting CLI publishing into its own workflow so symcc can be published independently between the base crates and the CLI.
Changes:
- Replace duplicated validation/publish steps in
publish.ymlandpublish_symcc.ymlwith a new reusable workflow (publish_reusable.yml). - Add a new
publish_cli.ymlworkflow to publishcedar-policy-cliseparately from the base crates. - Update base and symcc publish workflows to call the reusable workflow with appropriate tag/version settings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/publish.yml | Switches base-crate publishing to the reusable workflow and removes CLI publishing from this workflow. |
| .github/workflows/publish_symcc.yml | Switches SymCC publishing to the reusable workflow. |
| .github/workflows/publish_cli.yml | Adds a new standalone CLI publishing workflow that uses the reusable workflow. |
| .github/workflows/publish_reusable.yml | Introduces a reusable workflow encapsulating validation, version checks, and publishing logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Victor Nicolet <victornl@amazon.com>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Victor Nicolet <victornl@amazon.com>
victornicolet
force-pushed
the
victornicolet/release-workflow-split-cli-and-core
branch
2 times, most recently
from
25bc50f to
ddab8cb
Compare
Signed-off-by: Victor Nicolet <victornl@amazon.com>
victornicolet
force-pushed
the
victornicolet/release-workflow-split-cli-and-core
branch
from
ddab8cb to
a8153fb
Compare
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 100.00% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 86.81% Status: PASSED ✅ Details
|
.github/workflows/publish.yml
Outdated
| description: "The GitHub tag to be released. Must be of the form 'v<MAJOR>.<MINOR>.<PATCH>'" | ||
|
|
||
| description: "The core SDK tag. Must be of the form 'v<MAJOR>.<MINOR>.<PATCH>'. Required for core, cli, and all." | ||
| symcc_tag: |
There was a problem hiding this comment.
I think it makes more sense to have one tag and all of the crates to be published come from checking out that tag. If publishing core or all the tag should be of the form v<MAJOR>.<MINOR>.<PATCH>. If publishing just one crate (symcc or the cli) the tag should be <CRATE>-v<MAJOR>.<MINOR>.<PATCH>.
Then, if the target is "all" you need to specify the version of symcc and cli since they can't be extracted from the tag.
There was a problem hiding this comment.
I agree the current version is a little cumbersome because it forces you to always give the tag for core and sometimes the tag to symcc (if you're publishing symcc or all of them).
In the current workflow, all jobs will do at least a validate-core step. We could make the validation for the core tag more meaningful in symcc publishing by making symcc validate that the provided core tag is the version of the core on the provided symcc tag, according to the Cargo.toml.
I think having less implicit things (i.e. having to specify explicit tags, and two of them in some cases where you might only need one) is not necessarily a bad thing. But happy to change and make it easier to run
| -p cedar-policy \ | ||
| -p cedar-policy-cli | ||
| fi | ||
| # Order matters: dependencies must be listed before dependents. |
There was a problem hiding this comment.
I don't think the order actually matters if you specify packages in a single cargo publish command.
There was a problem hiding this comment.
I cannot find clear documentation that says that it does or it doesn't, and I can find some suggestions that it does. The previous workflow had the crates specified in the dependency order, so unless I'm proven wrong I'll leave it here.
| - core | ||
| - symcc | ||
| - cli | ||
| - all |
There was a problem hiding this comment.
It may make sense to have "all" be a separate workflow for all and put the shared logic in a composite GitHub action.
There was a problem hiding this comment.
Would that mean each main publish workflow is a composite Github action triggered by workflow_dispatch? Otherwise I don't know how well composite actions and workflow_dispatch mix.
There was a problem hiding this comment.
AFAIK composite actions can be called in the same way any action can. https://docs.github.com/en/actions/tutorials/create-actions/create-a-composite-action
The advantage of this over a reusable workflow is that it will run in the same job using the same runner as the caller. So you can have it do things like configure the environment.
There was a problem hiding this comment.
Pushed another "proposal" that still has all targets, each target runs 2 jobs, no reusable workflow. We could use composite action to remove the copy-pasted parts between each publish job?
victornicolet
changed the title
Signed-off-by: Victor Nicolet <victornl@amazon.com>
victornicolet
force-pushed
the
victornicolet/release-workflow-split-cli-and-core
branch
from
3e5bfe2 to
3944f43
Compare
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 100.00% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 86.81% Status: PASSED ✅ Details
|
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 100.00% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 86.81% Status: PASSED ✅ Details
|
Description of changes
Merges workflow for publishing core crates and symcc into one workflow with different targets:
corefor all core crates (i.e. not symcc and cli)symccclifor only the cli.allfor publishing all crates at once.The workflow requires a tag to decide where to publish from and at least the version of the core crates to check what is being published, or validate what core version symcc is being published on.
It requires explicit versions for what is being published depending on the target, on top of the tag that specifies what code to checkout for publishing.
Issue #, if available
Checklist for requesting a review
The change in this PR is (choose one, and delete the other options):
cedar-policy-core,cedarI confirm that this PR (choose one, and delete the other options):
I confirm that
cedar-spec(choose one, and delete the other options):I confirm that
docs.cedarpolicy.com(choose one, and delete the other options):