HYPERFLEET-1272 - refactor: rename query params pageSize→size and orderBy→order#63
Conversation
|
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: Enterprise Run ID: 📒 Files selected for processing (5)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
…erBy→order Aligns with api.openshift.com conventions. Removes unused OrderDirection enum and separate order direction parameter. Bumps version to 1.0.24.
1a18793 to
99f0717
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/core/openapi.yaml (1)
1420-1447: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGenerated
orderparameter now has noenum/pattern— same boundary gap flagged at source.The
QueryParams.orderparameter emits a baretype: stringwith noenumorpattern. This is the generated mirror of the root cause inshared/models/common/model.tsp:195-196; fixing the TypeSpec source and rebuilding will close it here. Not duplicating per-endpoint.Cross-repo: this is a breaking contract change. Per linked findings,
hyperfleet-sentinel/adapter/broker internal/client/client.go:347-435still sendPageSize, andclient_test.go:1253-1276assertpageSizein the URL — these will break until regenerated. Intended per PR objectives, but ensure downstream regeneration is tracked.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/core/openapi.yaml` around lines 1420 - 1447, The generated QueryParams.order schema is missing its allowed value constraints, so fix the TypeSpec source in shared/models/common/model.tsp where the order parameter is defined and regenerate the OpenAPI output. Add back the appropriate enum or pattern at the source rather than patching schemas/core/openapi.yaml directly, then rebuild so QueryParams.order emits the constrained string schema consistently across all generated endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@schemas/core/openapi.yaml`:
- Around line 1420-1447: The generated QueryParams.order schema is missing its
allowed value constraints, so fix the TypeSpec source in
shared/models/common/model.tsp where the order parameter is defined and
regenerate the OpenAPI output. Add back the appropriate enum or pattern at the
source rather than patching schemas/core/openapi.yaml directly, then rebuild so
QueryParams.order emits the constrained string schema consistently across all
generated endpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e4bf2e61-e5ee-4c9d-895f-564cc4725fca
📒 Files selected for processing (2)
schemas/core/openapi.yamlshared/models/common/model.tsp
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
e527445
into
openshift-hyperfleet:main
Summary
pageSizequery parameter tosizeorderByquery parameter toorderOrderDirectionenum and separateorder?: OrderDirectionparameterAligns with api.openshift.com conventions. This is a breaking change to the API contract, intended before GA.
Test plan
npm run buildcompiles successfullyschemas/core/openapi.yamlcontainssizeandorder(nopageSize,orderBy, orOrderDirection)Downstream PRs
Consumer repos will follow with their own PRs after this merges:
PageSize→Sizestruct fields) + tests