Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates CI to run example binaries only for crates impacted by the PR (as determined by cargo delta), reducing the cost of the “Examples” step while keeping a full-workspace backstop on non-PR runs.
Changes:
- Update
scripts/run-examples.ps1to accept multiple packages (string[]) and treat an empty list as “run for all workspace crates”. - Extend the
deltajob to emitaffected_names(space-separated impacted crate names). - Replace
just examplesin CI with a PowerShell step that parsesaffected_namesand invokesrun-examples.ps1for those crates (skipping entirely on PRs when no crates are affected).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
scripts/run-examples.ps1 |
Allows passing multiple crate names and derives the processed set from a non-empty package list. |
.github/workflows/main.yml |
Adds affected_names output from cargo delta and scopes the Examples step to only affected crates on PRs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #394 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 224 224
Lines 16187 16187
=======================================
Hits 16187 16187 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| $ErrorActionPreference = "Stop" | ||
| $PSNativeCommandUseErrorActionPreference = $true | ||
| $packages = @($env:AFFECTED_NAMES -split '\s+' | Where-Object { $_ }) |
There was a problem hiding this comment.
affected_names=$(impact -f names --affected | tr '\n' ' ') can yield a whitespace-only value if the impact command emits a trailing newline even when there are no affected crates. In that case, the Examples job condition (needs.delta.outputs.affected_names == '') won’t skip, and run-examples.ps1 will interpret an empty -Package list as “run all packages”, defeating the delta optimization. Consider trimming/normalizing affected_names in the compute step (e.g., remove leading/trailing whitespace) and/or adding a guard in the pwsh step to skip when $packages.Count -eq 0.
| $packages = @($env:AFFECTED_NAMES -split '\s+' | Where-Object { $_ }) | |
| $packages = @($env:AFFECTED_NAMES -split '\s+' | Where-Object { $_ }) | |
| if ($packages.Count -eq 0) { | |
| Write-Host "No affected packages; skipping examples." | |
| exit 0 | |
| } |
The tower example's inner service used `fastrand::i16(0..10) > 4` to simulate ~50%% failure rate, which made the example flaky on CI when retries happened to exhaust. Raise the threshold to `> 10` so the random branch can never fire, while leaving the surrounding retry/timeout pipeline intact as a demonstration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The per-example 30s timeout in run-examples.ps1 covered the cold cargo build/link cost as well, which made the first example for each package flake (e.g. `anyspawn::custom` timing out on Windows debug builds) once cargo-delta started invoking the script with narrower package sets that didn't share warm artifacts with prior workflow steps. Add a single `cargo build --examples` invocation for all selected packages before the timed loop. Each subsequent `cargo run` is then just a fingerprint check + exec, comfortably inside the 30s budget. Also surfaces real compile failures up front instead of disguising them as timeouts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Passing multiple `-p X -p Y` to cargo is ambiguous: cargo cannot reliably tell whether each name refers to a workspace member or a remote crate of the same name unless the package spec is fully qualified. The other workflow steps (cargo build/test/doc) avoid this by running `--workspace` with the `--exclude` list emitted by cargo-delta; the examples step should use the same plan. Reshape `run-examples.ps1` to accept `-CargoExcludes` (the raw cargo-style `--exclude foo --exclude bar` string already produced by `impact -f cargo-excludes`). It is forwarded verbatim to the pre-build cargo invocation alongside `--workspace`, and the same names drive the local discovery loop. `-Package` is retained as a mutually-exclusive single-package mode so `just package=foo examples` still works locally. On the YAML side this collapses the bespoke env/PowerShell preamble and the `affected_names` delta output to a single one-liner that mirrors the surrounding cargo steps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Symmetric with the existing -Package parameter (which is implicitly a 'cargo package' selector). The cargo-style `--exclude foo --exclude bar` raw-string semantics are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.