tool: update-rust-sdk.sh script, update rust hash - #254
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
|
|
||
| echo "==> Synchronizing nested Rust SDK submodules" | ||
| git -C "$rust_dir" submodule sync --recursive | ||
| git -C "$rust_dir" submodule update --init --recursive |
There was a problem hiding this comment.
🟡 Nested submodules remain on wrong commits
With a local rebase, merge, or none update mode, submodule update does not check out the pinned commit. The script reports success while builds use different Rust dependencies.
| git -C "$rust_dir" submodule update --init --recursive | |
| git -C "$rust_dir" submodule update --init --recursive --checkout |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are low-risk and self-contained (a new helper script plus doc updates), with only minor documentation/help-text clarifications suggested.
Pull request overview
Adds a new helper script to update the client-sdk-rust submodule (either to the latest published Rust SDK release or to a user-specified commit), and updates developer documentation to use this script for Rust submodule bumps.
Changes:
- Introduce
scripts/update-rust-sdk.shto automate selecting/fetching/checking out the Rust SDK submodule revision and syncing nested submodules. - Update
docs/tools.mdto document the new scripted workflow for bumping the pinned Rust submodule.
File summaries
| File | Description |
|---|---|
| scripts/update-rust-sdk.sh | New automation script to update the Rust submodule and recursively sync/update its nested submodules. |
| docs/tools.md | Replaces the manual Rust submodule bump steps with instructions for using the new update script. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| The script requires the GitHub CLI (`gh`). To select a specific commit, pass | ||
| its full or abbreviated hash: |
| When --hash is omitted, the script uses the commit tagged by the most recent | ||
| published, non-draft GitHub release in livekit/client-sdk-rust. |
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| release_tags="$( | ||
| gh api --paginate "repos/${rust_repo}/releases?per_page=100" \ | ||
| --jq '.[] | select(.draft == false) | .tag_name' | ||
| )" | ||
| release_tag="${release_tags%%$'\n'*}" |
There was a problem hiding this comment.
🟡 Newest publication can be skipped
When creation and publication order differ, release_tags selects the API's first non-draft release. It can pin an older publication.
Prompt for agents
The default update path in scripts/update-rust-sdk.sh assumes the first non-draft item returned by GitHub is the most recently published release. GitHub's release listing order is not publication-time order, so a release published later from an older-created commit can be skipped. Collect all non-draft releases with their published_at timestamps, select the maximum publication timestamp, then use that release's tag. Preserve pagination and fail clearly when no published release exists.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo "==> Fetching client-sdk-rust" | ||
| if [[ "$hash_provided" == false ]]; then | ||
| git -C "$rust_dir" fetch --quiet origin "$fetch_ref" | ||
| elif [[ ${#requested_hash} -eq 40 ]]; then | ||
| if ! git -C "$rust_dir" fetch --quiet origin "$fetch_ref"; then | ||
| git -C "$rust_dir" fetch --quiet origin | ||
| fi | ||
| else | ||
| git -C "$rust_dir" fetch --quiet origin | ||
| fi |
There was a problem hiding this comment.
🟡 Customized remotes block updates
When a submodule's origin targets a fork, fetch searches that fork for the canonical commit. Both update commands then fail.
Prompt for agents
scripts/update-rust-sdk.sh discovers releases from rust_remote but fetches from the mutable origin remote. scripts/update-cpp-example-collection.sh has the same mismatch with examples_remote. A user-configured fork or mirror can therefore lack the commit selected from the canonical repository. Make discovery and fetching use the same canonical source while retaining support for abbreviated hashes, which requires fetching enough canonical refs for disambiguation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Overview