fix(deps): hold both windows names, not just windows-core majors - #652
Conversation
I claimed the rule already reached a lone 0.63 because Dependabot classifies a 0.x minor-position bump as semver-major, and cited this repository's history as proof. That inference was too strong. What the history shows is how `groups` behaves — no 0.x bump of that shape has ever landed inside cargo-patch-and-minor — and `groups` and `ignore` are not obliged to classify alike. GitHub documents SemVer as plain major.minor.patch, which makes 0.62 -> 0.63 a minor, and nothing authoritative settles the 0.x case either way. So stop betting on the classification. Both names are now held back from major and minor alike, which is correct under either reading, and the mirrored failure the previous rule risked is gone with it: `windows` was left free, so a paired 0.63 would have bumped `windows` alone and desynchronised in the other direction — the same defect as #642 with the two crates swapped. Patches stay automatic and are safe by construction: both names remain inside 0.62.x, where Cargo unifies them onto one version regardless. Accepted cost, written at the rule: an ignore can also hold back a security update needing a minor bump.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Limit details: You’ve used the included review currently available. Your 89 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughLa configuration Dependabot bloque les mises à jour majeures et mineures de ChangesAlignement des dépendances Windows
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The dependency update policy now consistently requires coordinated manual minor and major upgrades for both Windows crates. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Usage-based review receipt
Note This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing. Comment |
Follow-up to #650, which landed one revision early. The rule it merged holds back
windows-coremajors only, and leaveswindowsfree — which is not enough, and in one scenario is worse than nothing.What #650 got wrong
I argued there that the rule already reached a lone
windows-core0.63, because Dependabot classifies a 0.x minor-position bump as semver-major, and I offered this repository's history as proof: no 0.x bump of that shape has ever landed insidecargo-patch-and-minor— quick-xml 0.41 → 0.42, lofty 0.24 → 0.25, wasapi 0.23 → 0.24, resvg 0.47 → 0.48, rtrb 0.3 → 0.4, rfd 0.16 → 0.17.That evidence is real but it is about
groups, and the conclusion was aboutignore. The two keys are not obliged to classify alike. GitHub documents SemVer as plainmajor.minor.patch, which makes 0.62 → 0.63 a minor, and nothing authoritative settles the 0.x case either way. The claim was an inference presented as a fact.The worse half
Leaving
windowsout of the rule was justified as keeping a signal: the day it offers a major is the day the pair can move. Under the documented reading that backfires. A coordinated 0.63 would classify as minor for both names,windows-corewould be ignored, andwindowswould be swept intocargo-patch-and-minoron its own — desynchronising the pair in the other direction and reproducing #642 with the two crates swapped. The guard would have caused the defect it was written to prevent.What this does instead
Stops betting on the classification. Both names are held back from
semver-majorandsemver-minoralike, which is correct under either reading, and the pair is bumped by hand in one commit when windows-rs realigns.Patches stay automatic and are safe by construction: both names remain inside 0.62.x, which Cargo unifies onto a single version regardless of what Dependabot proposes.
Accepted cost, written at the rule so it is not rediscovered as a surprise: an
ignorecan also hold back a security update that needs a minor bump. Patch-level advisories still flow, and a desync is a build failure on every Windows job, which a slightly stale binding is not.Still not solved
This suppresses a proposal; it does not verify the invariant. A
windows/windows-coredesync reachingCargo.lockby any other route still surfaces as a wall of trait errors on the Windows job rather than as a named failure. The thing that would enforce it is an offlineCargo.lockcheck in CI, in the shape ofcheck-toolchain-pin.pyandcheck-sources.py— deliberately not in this PR, and worth its own issue.Validation
Local CodeRabbit pass: 0 findings. YAML parses; the rule reads back as two entries, each carrying both update types.
Summary by CodeRabbit
windowsetwindows-coresont désormais effectuées manuellement et conjointement.