Replace dead feature-powerset CI job with curated feature check#1601
Replace dead feature-powerset CI job with curated feature check#1601Jarema wants to merge 2 commits into
Conversation
The check_features job ran cargo hack --feature-powerset, which exceeds the 6h GitHub Actions cap and has been cancelled on every push to main — feature-combination checking never actually passed, while burning 6h of CI per push. Use the curated .cargo-hack-check.sh instead (written for exactly this), extend it to cover the recently added server_2_14 default feature plus fips and experimental, add a 45-minute job timeout, and run the check on pull requests too so feature-gated breakage is caught before merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @Jarema's task in 2m 26s —— View job PR Review
ReviewThe fix is correct and the approach is sound. Replacing a powerset that mathematically cannot complete (~18 features → billions of combinations, always hitting the 6h cap) with a curated set is the right call. No bugs, security issues, or correctness problems found. Coverage analysis of the script:
One minor observation — The new This isn't introduced by the PR (the old powerset job also lacked Stale comment (preexisting, not introduced here): The changes are ready to merge as-is. The RUSTFLAGS parity and stale comment can be a follow-up. |
Signed-off-by: Tomasz Pietrek <tomasz@synadia.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 505ea054b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| let retry_strategy = tokio_retry::strategy::ExponentialBackoff::from_millis(500).take(3); | ||
| let mut stream = tokio_retry::Retry::spawn(retry_strategy, || { | ||
| let mut stream = tokio_retry::Retry::start(retry_strategy, || { |
There was a problem hiding this comment.
Keep tokio-retry API within declared version range
This call depends on the just-added Retry::start API, but nats-server/Cargo.toml still declares tokio-retry = "0.3.0", so Cargo may legally resolve 0.3.0 or 0.3.1 from an existing/minimal lockfile. I checked the docs.rs API for those versions and they expose Retry::spawn but not start; start only appears in 0.3.2. In those environments the nats-server test package fails to compile before this retry is ever exercised, so either keep using spawn or raise the dependency lower bound to a version that has start.
Useful? React with 👍 / 👎.
Problem
The
check_featuresjob inmain.ymlrunscargo hack check --feature-powerset. With ~18 features the powerset explodes into hundreds of thousands of combinations, and the job has been cancelled at the 6h GitHub Actions cap on every recent push to main — feature-combination checking has never actually passed, while burning 6h of CI per push. Meanwhileasync-nats/.cargo-hack-check.sh(written precisely to avoid the explosion) was referenced by no workflow.This is exactly how feature-gated breakage can ship undetected (e.g. cfg-gated builder code only compiled under specific flag sets).
Changes
main.yml:check_featuresnow runsbash .cargo-hack-check.shwithtimeout-minutes: 45instead of the powerset.test.yml: newcheck_featuresjob — feature combinations are now checked on pull requests, not only after merge..cargo-hack-check.sh: extended for recent feature additions —server_2_14(now in defaults),fips, andexperimental. Chrono-backend combinations were already covered.Verification
Full script run locally with
RUSTFLAGS="-D warnings": all strategies pass (--each-featurewith ring, curated combinations incl. chrono backend, aws-lc-rs/fips, all server version flags, experimental).🤖 Generated with Claude Code