Skip to content

Navigation Menu

Sign in
Sign up

tool: update-rust-sdk.sh script, update rust hash - #254

Open
stephen-derosa wants to merge 2 commits into
main from
sderosa/update-commit
Open

tool: update-rust-sdk.sh script, update rust hash #254
stephen-derosa wants to merge 2 commits into
main from
sderosa/update-commit

Conversation

@stephen-derosa

@stephen-derosa stephen-derosa commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Overview

  • helper script to update to latest rust hash
  • new rust hash has webrtc changes, must run stress tests

Copilot AI lite review requested due to automatic review settings September 3, 2026 14:42

@devin-ai-integration devin-ai-integration Bot left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Devin Review


echo "==> Synchronizing nested Rust SDK submodules"
git -C "$rust_dir" submodule sync --recursive
git -C "$rust_dir" submodule update --init --recursive

@devin-ai-integration devin-ai-integration Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Suggested change
git -C "$rust_dir" submodule update --init --recursive
git -C "$rust_dir" submodule update --init --recursive --checkout
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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.sh to automate selecting/fetching/checking out the Rust SDK submodule revision and syncing nested submodules.
  • Update docs/tools.md to 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.

Comment thread docs/tools.md
Comment on lines +149 to +150
The script requires the GitHub CLI (`gh`). To select a specific commit, pass
its full or abbreviated hash:
Comment on lines +38 to +39
When --hash is omitted, the script uses the commit tagged by the most recent
published, non-draft GitHub release in livekit/client-sdk-rust.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

Comment on lines +111 to +115
release_tags="$(
gh api --paginate "repos/${rust_repo}/releases?per_page=100" \
--jq '.[] | select(.draft == false) | .tag_name'
)"
release_tag="${release_tags%%$'\n'*}"

@devin-ai-integration devin-ai-integration Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +146 to +155
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

@devin-ai-integration devin-ai-integration Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments
Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@xianshijing-lk xianshijing-lk Awaiting requested review from xianshijing-lk xianshijing-lk is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /