chore: move all remaining external deps to workspace-level dependencies - #2476
chore: move all remaining external deps to workspace-level dependencies#2476yannham wants to merge 3 commits into
Conversation
…evel Move all external (non-inter-workspace) dependencies declared directly in member crates to [workspace.dependencies], with default features disabled per the workspace convention. Leaf crates reference them via workspace = true and opt into the features they use.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🔒 Cargo Deny Results📦
|
There was a problem hiding this comment.
More details
Source inspection finds no reportable defect in the dependency changes, feature selections, or macro syntax updates.
🤖 Datadog Autotest · Commit 737075e · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
📚 Documentation Check Results📦
|
|
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
BenchmarksComparisonBenchmark execution time: 2026-09-09 10:25:57 Comparing candidate commit c56110d in PR branch Found 10 performance improvements and 7 performance regressions! Performance is the same for 136 metrics, 0 unstable metrics.
|
| # `std` is required: it gates the alloc-backed `ByteBuf`/`Vec<u8>` serde impls used by | ||
| # `#[serde(with = "serde_bytes")]` in the generated code. |
There was a problem hiding this comment.
I'm not sure this is needed, I don't think the crate is supposed to no-std anyway
There was a problem hiding this comment.
Sorry, what do yo think is not needed? The comment? The default-features?
There was a problem hiding this comment.
The comment is mentioning std which is a bit confusing (not clear it's talking about the feature). I guess it's fine to explain why we add the default feature it just seems weird that it's the only dependency explaining it.
There was a problem hiding this comment.
Ah, got it. I think it's because elsewhere I've avoided std features on many crates, preferring alloc, in the spirit of the lint we have (make it easier to move crates to no-std in the future). Though the comment is dubious because serde-bytes does have an alloc feature. I'll either remove it if I can get by with alloc, or reword it to make things clearer 👍
There was a problem hiding this comment.
More details
The workspace dependency move keeps each leaf crate's feature choice. The required duplicate, priority-queue, and elf API changes match the new dependency versions.
🤖 Datadog Autotest · Commit c56110d · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
What does this PR do?
Follow-up of #2350. Migrate all of the remaining external dependencies to the workspace level.
duplicatewas used with incompatible major versions. Mostly the macro syntax changed. Files are updated to use the latest version's syntax.Motivation
As for the other workspace-level deps: use a single source of truth and version, for each dependency, in the whole workspace. Force leaf crates to opt-in features they need instead of blindly enabling default features.