refactor(core): derive Default for CubeQuery - #2722
Conversation
Callers had to name every field of the struct literal, so #2677 adding orderBy was a source-breaking change for anyone constructing a CubeQuery directly. Every field already satisfies Default, so the derive is free and lets callers write `..Default::default()` — future optional fields then stay additive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. Walkthrough
ChangesCubeQuery default construction
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change makes CubeQuery construction more compatible without changing query validation, SQL generation, or runtime behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the change, motivation, absence of behavioral changes, validation steps, and mutation check. It does not include an explicit Duplicate check section, but the description is otherwise complete and directly related to the pull request.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
What changed
DefaultforCubeQueryCubeQueryvia..Default::default()in the two existing struct literalsWhy
Follow-up on the review of #2677.
CubeQueryis a public type with all-public fields, so a struct literal has to name every field. AddingorderByin #2677 was therefore a source-breaking change for anyone constructing aCubeQuerydirectly.Every field already satisfies
Default, so the derive costs nothing and lets callers name only the fields they care about:Future optional fields then stay additive rather than breaking downstream builds.
No behavioural change:
#[derive(Default)]only generatesimpl Default for CubeQuery. Field-level#[serde(default)]already resolved against each field'sDefault, so deserialization is untouched.Validation
cargo fmt --all -- --checkcargo check --all-targetscargo clippy --all-targets --all-features -- -D warningscargo test --lib --tests --bins— lib 160 passed,cube_public_api1 passed, sqllogictests passedwren-core-py:cargo test37 passed,maturin develop+pytest46 passed (incl. 13 cube binding tests)error[E0277]: the trait boundCubeQuery: Defaultis not satisfiedatcore/src/mdl/cube.rs:654andcore/tests/cube_public_api.rs:34.Summary by CodeRabbit