Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The cleanup is internally consistent, preserves required environment handling, and includes appropriate validation.
Review tier: Balanced
Findings: None
What changed in this PR
Removes obsolete Rust toolchain overrides now that toolchain pinning is managed independently.
Changes:
- Removes fixed-toolchain selection, logging, and preinstallation.
- Uses standard
cargowhile preserving configured environment variables. - Removes the direct
ra_ap_toolchaindependency and regenerates Bazel metadata.
| File | Description |
|---|---|
rust/ql/test/setup.sh |
Stops preinstalling Rust 1.97. |
rust/ql/integration-tests/conftest.py |
Removes integration-test toolchain installation. |
rust/extractor/src/toolchain.rs |
Deletes fixed-toolchain logic. |
rust/extractor/src/qltest.rs |
Runs standard cargo with configured environment. |
rust/extractor/src/main.rs |
Removes toolchain logging and module registration. |
rust/extractor/src/config.rs |
Stops overriding RUSTUP_TOOLCHAIN. |
rust/extractor/Cargo.toml |
Removes the direct toolchain dependency. |
MODULE.bazel |
Removes the obsolete direct Bazel repository import. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/ra_ap_toolchain/BUILD.bazel |
Deletes obsolete generated alias. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/ra_ap_toolchain-0.0.347/BUILD.bazel |
Deletes versioned generated alias. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl |
Regenerates direct dependency metadata. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel |
Removes top-level toolchain aliases. |
Cargo.lock |
Removes the extractor’s direct dependency entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Drop the synthetic format-family macro expansion path and its dedicated legacy-toolchain tests now that extraction uses a pinned compatible Rust toolchain. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
c51a5eb to
2241a07
Compare
|
Thanks for starting DCA correctly. There are some extraction errors. I'll investigate. |
|
Extraction errors are for DCA projects on an old nightly toolchain. For these projects the format-macro fallback is in fact not dead code.format-macro fallback is not dead code. I'm putting back in draft to re-consider. We could go ahead as-is, but it might be better just to implement #22493 (comment) to avoid noise in QA. |
Context
This PR
The PR essentially just reverts 7faf81f, but it's not an exact revert, since some later commits tweaked things a bit.
Spelled out the PR: