Conversation
Co-authored-by: harsha-simhadri <5590673+harsha-simhadri@users.noreply.github.com>
Co-authored-by: harsha-simhadri <5590673+harsha-simhadri@users.noreply.github.com>
Co-authored-by: harsha-simhadri <5590673+harsha-simhadri@users.noreply.github.com>
Co-authored-by: harsha-simhadri <5590673+harsha-simhadri@users.noreply.github.com>
Co-authored-by: harsha-simhadri <5590673+harsha-simhadri@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #755 +/- ##
==========================================
+ Coverage 88.99% 89.11% +0.11%
==========================================
Files 428 443 +15
Lines 78234 83354 +5120
==========================================
+ Hits 69626 74281 +4655
- Misses 8608 9073 +465
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds an automated crate publishing workflow for releasing DiskANN crates to crates.io. The workflow can be triggered manually via workflow_dispatch with a dry-run option (default: true) for testing, or automatically when pushing version tags matching v{major}.{minor}.{patch}. The implementation aims to enable safe testing of the release process without actual publication.
Changes:
- Added
.github/workflows/publish.ymlworkflow with dual-trigger support (tags and manual dispatch) - Added
.github/PUBLISH_CRATES.mddocumentation describing the release process and testing procedures - Implemented dry-run mode for testing packaging and validation without publishing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
.github/workflows/publish.yml |
New GitHub Actions workflow that validates versions, runs tests, and publishes workspace crates with optional dry-run mode |
.github/PUBLISH_CRATES.md |
Comprehensive documentation covering prerequisites, dry-run testing, release steps, and pre-release checklist |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/publish.yml
Outdated
| CARGO_REGISTRY_TOKEN: ${{ secrets.CRATES_IO_TOKEN }} | ||
| run: | | ||
| DRY_RUN_FLAG="" | ||
| if [ "${{ github.event.inputs.dry_run || 'false' }}" = "true" ]; then |
There was a problem hiding this comment.
The dry-run default expression github.event.inputs.dry_run || 'false' will evaluate to 'false' when the workflow is triggered by a tag push (since github.event.inputs.dry_run will be empty/null). This is correct behavior for tag-triggered publishes. However, consider making this more explicit with a comment or using a more readable approach like setting a workflow-level environment variable that checks both the trigger type and input.
| if [ "${{ github.event.inputs.dry_run || 'false' }}" = "true" ]; then | |
| # For tag-push events, github.event.inputs.dry_run is empty, so this evaluates to a real (non-dry-run) publish. | |
| if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then |
.github/PUBLISH_CRATES.md
Outdated
| cargo search diskann --limit 20 | ||
| ``` | ||
|
|
||
| ### Example Pre-Release Flow |
There was a problem hiding this comment.
This recommends pushing directly to main, which we can't do due to branch protection rules.
Instead, we should use this workflow:
- Checkout main, bump the version and push.
- Make a pull-request out of the version bumped commit.
- Use the dry-run as a pre-merge check.
- Merge the PR and use the github UI to tag the release along with change notes.
There was a problem hiding this comment.
I think I addressed this oversight
|
|
||
| ### What It Does NOT Test | ||
|
|
||
| - Actual publishing, registry token auth, upload reliability |
There was a problem hiding this comment.
We may want to mention what to do if the publish step fails. Basically, we either need to manually fix and publish the remaining crates, or fix the publish issue, bump the version number, and bump again (or if the fix doesn't change the versions that succeeded, then the remaining ones can be updated and pushed potentially in the same release).
If the publishing fails part way through for networking issues, then we'll need to handle the remaining ones manually.
There was a problem hiding this comment.
I am not certain of the behavior. Coudl it be that running the action again uploads missing crate? can we handle this when it happens?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
||
| ```bash | ||
| git checkout main && git pull | ||
| git checkout -b release-0.46.0 |
There was a problem hiding this comment.
This conflicts with the tags in the PR description and the comments of the workflow yaml. I assume this one should be v0.46.0.
There was a problem hiding this comment.
it is 0.46.0 right? Perhaps I pushed after your comment?
.github/PUBLISH_CRATES.md
Outdated
| version = "0.46.0" | ||
| ``` | ||
|
|
||
| All workspace crates inherit this via `version.workspace = true`. |
There was a problem hiding this comment.
Don't all the other crate versions also need updating?
There was a problem hiding this comment.
I think all creates are inheriting via version.workspace=true.
Do you mean we should update the versions under workspace.dependcies? I can add a check to ensure they all align with the version being published if thats what you meant?
There was a problem hiding this comment.
I added a step in publishing instructions to advise.
shall I add something like this to the CI to automate testing
- name: Verify workspace dependency versions match workspace.package.version
run: |
WORKSPACE_VERSION=$(sed -n '/\[workspace\.package\]/,/^\[/{ s/^version *= *"\(.*\)"/\1/p }' Cargo.toml)
echo "workspace.package.version = $WORKSPACE_VERSION"
mismatched=()
while IFS= read -r line; do
crate=$(echo "$line" | sed 's/\s*=.*//')
if echo "$line" | grep -q "path\s*=" && echo "$line" | grep -qv "version\s*=\s*\"$WORKSPACE_VERSION\""; then
mismatched+=("$crate")
fi
done < <(sed -n '/\[workspace\.dependencies\]/,/^\[/{/^\[/d;/^#/d;/^$/d;p}' Cargo.toml)
if [ ${#mismatched[@]} -gt 0 ]; then
echo "::error::The following workspace dependencies have a version that does not match $WORKSPACE_VERSION: ${mismatched[*]}"
exit 1
fi
```
|
|
||
| ```bash | ||
| git commit -am "Bump version to 0.46.0" | ||
| git push origin release-0.46.0 |
There was a problem hiding this comment.
sorry, is the version syntax wrong or the concrete number wrong? this is just an example, right?
|
|
||
| 6. **Merge the PR** into `main`. | ||
|
|
||
| 7. **Create a release** via the GitHub UI: |
There was a problem hiding this comment.
You mean publish crate whenever a PR with new version is merged to main? Possibly. Being a bit conservative for now
| name: >- | ||
| ${{ | ||
| github.event_name == 'pull_request' | ||
| && 'Dry-run publish test' |
There was a problem hiding this comment.
What do these strings evaluate as or do? I don't understand the test condition.
There was a problem hiding this comment.
removed unnecessary clauses
The automated publish workflow could only be triggered by pushing version tags, making it impossible to test changes to the workflow or validate a release without actually publishing to crates.io.
Changes
Workflow trigger
workflow_dispatchwithdry_runinput (defaults totrue)Publish logic
--dry-runflag tocargo publishwhen enabled🧪 DRY-RUN MODEvs📦 LIVE MODEDocumentation
RELEASING.md: Testing section with dry-run vs live comparison.github/TESTING_RELEASES.md: Step-by-step testing guideUsage
Manual test without publishing:
Actual publish (unchanged):
git tag v0.46.0 && git push origin v0.46.0Dry-run validates packaging, dependencies, and publish order across all 15 crates without uploading to the registry.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.