Rust: aggregate projects into workspaces #18500
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Exact data can be seen here (internal link only available to staff). |
Contributor
There was a problem hiding this comment.
Copilot reviewed 15 out of 31 changed files in this pull request and generated no comments.
Files not reviewed (16)
- MODULE.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.serde_spanned-0.6.8.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.toml-0.8.19.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.toml_datetime-0.6.8.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.toml_edit-0.22.22.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.winnow-0.6.24.bazel: Language not supported
- misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl: Language not supported
- rust/ql/.generated.list: Language not supported
- rust/ql/integration-tests/hello-project/diagnostics.expected: Language not supported
- rust/ql/integration-tests/hello-project/steps.cargo.expected: Language not supported
- rust/ql/integration-tests/hello-project/steps.rust-project.expected: Language not supported
- rust/ql/integration-tests/hello-project/summary.expected: Language not supported
- rust/ql/integration-tests/hello-workspace/diagnostics.cargo.expected: Language not supported
- rust/ql/integration-tests/hello-workspace/diagnostics.rust-project.expected: Language not supported
- rust/ql/integration-tests/hello-workspace/steps.cargo.expected: Language not supported
Comments suppressed due to low confidence (4)
rust/extractor/src/rust_analyzer.rs:167
- [nitpick] The error message could be more descriptive. Consider providing more context about the error.
return Err(anyhow::anyhow!("{manifest} not a cargo manifest"));
rust/extractor/src/rust_analyzer.rs:216
- Using
unwrap_orcan potentially cause a panic. Consider handling the error more gracefully.
let workspace = find_workspace(&mut reader, &manifest).unwrap_or(manifest);
rust/extractor/src/rust_analyzer.rs:134
- Ensure that there are tests covering the new behavior introduced by the
ToMlReaderstruct and its methods.
struct ToMlReader {
rust/extractor/src/rust_analyzer.rs:204
- Ensure that there are tests covering the new behavior introduced by the
find_project_manifestsfunction.
pub fn find_project_manifests(
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
aibaars
reviewed
Jan 17, 2025
rust/extractor/src/rust_analyzer.rs
Outdated
| workspace: Option<CargoManifestMembersSlice>, | ||
| } | ||
|
|
||
| struct ToMlReader { |
Contributor
Author
There was a problem hiding this comment.
ah, typo because of a brain fart 😆
aibaars
previously approved these changes
Jan 17, 2025
Contributor
aibaars
left a comment
aibaars
left a comment
There was a problem hiding this comment.
Looks good to me. Just wondering why you wrote ToMl?
Contributor
Author
Fixed now, mind reapproving? |
aibaars
approved these changes
Jan 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds logic to pick up
Cargo.tomlworkspaces over its members, loading a whole workspace intorust-analyzerinstead of its members one by one. On our DCA sources this is showing a ~34% average database extraction speedup. The actual speedup is a bit spread out, from a minimum of ~0.6% to a maximum of -75%. The median is at ~25%. More importantly though, no project has seen any slowdown, and the slowest e2e timing in our DCA sources has seen a 37% reduction.