fix(derive): name the count type in standing presence checks - #1205
Conversation
`verbose: u8` with `count` generated `!= Default::default()`, which is ambiguous when serde_json's PartialEq<Value> for u8 is also in scope. hk's CLI failed to compile against the update_from standing code from #1197 for that reason. Name the field type so the comparison stays u8. Co-authored-by: jdx <jdx@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change resolves count-field default comparisons through each field’s declared type. It adds a conformance fixture and regression test for counted flags during updates with subcommands and ChangesCount update handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized derive fix makes standing presence checks use the generated count field's declared type, avoiding an ambiguity with serde_json comparisons; no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes. Shadow comparisonParsing
|
Summary
update_fromstanding presence forcountfields generated!= Default::default(). When an adopter also hasserde_jsonin the same crate,PartialEq<Value> for u8makes that comparison ambiguous and the derive fails to compile.hk's CLI (
verbose: u8withcount, pluscompletion/ globals / overrides) hits this on current main after #1197. Name the field type so the comparison staysu8.This is a blocker for repinning the fleet clap→usage PRs onto current main for the v6 swap.
Fleet readiness (companion)
Reviewed and locally checked against this revision:
hk.usage.kdl(flagset)completionflag order (force/install); patch readyThis agent only has write access to
jdx/usage, so fleet PR updates could not be pushed from here.Test plan
a_standing_count_compiles_beside_serde_jsonupdate_fromsuite greencompletion+ global count + overrides +serde_jsoncompileshkonagent/usage-6-experimentcompiles against this revisionSummary by CodeRabbit
u8flags so their existing values remain unchanged when no new value is provided.