Conversation
1906d9e to
ff749c9
Compare
There was a problem hiding this comment.
Pull Request Overview
Centralize the Rust nightly version in rust-toolchain.toml and update CI to consume that single source of truth
- Added
rust-toolchain.tomlwithchannel = "nightly-2025-02-28" - Removed hardcoded
RUST_NIGHTLY_VERSIONand the dedicateddtolnay/rust-toolchainstep - Introduced extraction of the channel from TOML and changed the Windows installer to use
--default-toolchain none
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust-toolchain.toml | Pin nightly channel to 2025-02-28 |
| .github/workflows/ci.yml | Remove hardcoded version, adjust Windows install command, extract version from TOML |
Files not reviewed (1)
- Dockerfile: Language not supported
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:473
- Removing this action also removed component installation (
rustfmt,clippy,miri), which could break formatting and lint checks. Consider re-adding component installation viarustup component addor an equivalent step.
- - uses: dtolnay/rust-toolchain@83bdede770b06329615974cf8c786f845d824dfb # nightly
There was a problem hiding this comment.
@brandonmarken can i have you review on this one too ?
This PR propagates the rust nightly pinning everywhere, through a rust-toolchain.toml
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes the pinned Rust nightly version in rust-toolchain.toml and updates the CI workflow to read from that file instead of an env var.
- Add
rust-toolchain.tomlwithchannel = "nightly-2025-02-28" - Remove hardcoded
RUST_NIGHTLY_VERSIONand thedtolnay/rust-toolchainstep from CI - Update Windows install and add a script snippet to extract the nightly date from the toolchain file
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust-toolchain.toml | Add [toolchain] channel pinned to nightly-2025-02-28 |
| .github/workflows/ci.yml | Remove hardcoded version, adjust rustup invocation, extract version from TOML |
Files not reviewed (1)
- Dockerfile: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/ci.yml:473
- Removing the
dtolnay/rust-toolchainstep also drops installation ofrustfmt,clippy, andmiri. CI will no longer run formatting or lint checks—re-add component installation or replace it with another action that installs these tools.
- uses: dtolnay/rust-toolchain@83bdede770b06329615974cf8c786f845d824dfb # nightly
.github/workflows/ci.yml:578
- The
--default-toolchainflag was removed from the Windows install command, so the nightly toolchain won’t be set and default to stable. Add--default-toolchain $(grep '^channel' rust-toolchain.toml | cut -d '"' -f 2)or similar to ensure the correct toolchain is installed.
C:\rustup-init.exe --default-host x86_64-pc-windows-gnu -y
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes the Rust toolchain version in rust-toolchain.toml and updates the CI workflow to read from that file rather than hardcoding the nightly date.
- Adds
rust-toolchain.tomlwith the pinned nightly channel. - Removes hardcoded
RUST_NIGHTLY_VERSIONin CI and replaces it by parsingrust-toolchain.toml. - Adjusts Windows setup and build steps to use the extracted channel.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust-toolchain.toml | Introduced as single source of truth for Rust channel. |
| .github/workflows/ci.yml | Removed hardcoded nightly env and action; added Python parsing of TOML and updated steps to use extracted channel. |
Files not reviewed (2)
- .github/builder/common.sh: Language not supported
- Dockerfile: Language not supported
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:660
- GitHub Actions does not natively support Python as a shell using this syntax. Consider running the parser via a standard shell step, e.g.:
run: python3 - <<EOF ... EOFor an external script file.
shell: python3 {0}
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes the Rust nightly channel in rust-toolchain.toml and updates the CI workflow to extract and use that single source of truth.
- Add
rust-toolchain.tomlwith pinned nightly date - Remove hard-coded nightly version from CI and introduce Python parsing of the toolchain file
- Update CI steps to pass the extracted nightly date into the build process
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust-toolchain.toml | Pin nightly channel to 2025-02-28 |
| .github/workflows/ci.yml | Remove hard-coded version, parse rust-toolchain.toml, adjust setup steps |
Files not reviewed (2)
- .github/builder/common.sh: Language not supported
- Dockerfile: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/ci.yml:473
- Removing the Rust toolchain setup action drops installation of required components (
rustfmt,clippy,miri) in CI. Consider adding a setup step that installs these using the channel fromrust-toolchain.toml.
- uses: dtolnay/rust-toolchain@83bdede770b06329615974cf8c786f845d824dfb # nightly
.github/workflows/ci.yml:671
- [nitpick] The output key
nightly_datedoesn’t align with the other CI variable naming (RUST_*). Consider renaming it torust_nightly_dateor matching the uppercase style for consistency.
out.write(f'nightly_date={date}\n')
.github/workflows/ci.yml
Outdated
| Invoke-WebRequest -URI https://win.rustup.rs/x86_64 -OutFile C:\rustup-init.exe | ||
| echo "Installing Rust" | ||
| C:\rustup-init.exe --default-toolchain nightly-${{ env.RUST_NIGHTLY_VERSION }} --default-host x86_64-pc-windows-gnu -y | ||
| C:\rustup-init.exe --default-host x86_64-pc-windows-gnu -y |
There was a problem hiding this comment.
The Windows installation step no longer specifies the nightly toolchain, so rustup will default to stable. Add --default-toolchain ${{ steps.rust-channel.outputs.nightly_date }} to install the correct nightly version.
| C:\rustup-init.exe --default-host x86_64-pc-windows-gnu -y | |
| C:\rustup-init.exe --default-host x86_64-pc-windows-gnu --default-toolchain ${{ steps.rust-channel.outputs.nightly_date }} -y |
ed71a24 to
bd0f812
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR pins the Rust nightly toolchain to 2025-02-28 by using a new rust-toolchain.toml file as the single source of truth and updates the CI workflow accordingly.
- Introduces rust-toolchain.toml with the specified nightly channel.
- Removes the hardcoded RUST_NIGHTLY_VERSION environment variable and dtolnay/rust-toolchain action from the CI workflow.
- Extracts the Rust channel from rust-toolchain.toml using a Python step and passes this to build commands.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust-toolchain.toml | Adds a [toolchain] section specifying the nightly channel. |
| .github/workflows/ci.yml | Updates CI steps to remove the RUST_NIGHTLY_VERSION env variable and extracts the channel from rust-toolchain.toml for use in subsequent steps. |
Files not reviewed (2)
- .github/builder/common.sh: Language not supported
- Dockerfile: Language not supported
brandonmarken
left a comment
There was a problem hiding this comment.
All looks good to me!
Use
rust-toolchain.tomlas single source of truth