-
Notifications
You must be signed in to change notification settings - Fork 22
refactor(specs): group files for composition api #5122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✔️ Code generated!
📊 Benchmark resultsBenchmarks performed on the method using a mock server, the results might not reflect the real-world performance.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still differ from the regular search params? If not, we can remove it and use the search spec instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter are the same than the Search API but the usahe is not allowed in all part of the API.
We are way more restricted.
I will try to clarify the usage in my next PR
specs/composition-full/common/schemas/components/CompositionRule.yml
Outdated
Show resolved
Hide resolved
specs/composition-full/common/schemas/components/CompositionRule.yml
Outdated
Show resolved
Hide resolved
type: object | ||
additionalProperties: false | ||
properties: | ||
attributesToRetrieve: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if those are expected to be exactly the same as the one from the search API, you can import them from either the Search Params or the Index Settings, see recommend spec example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah maybe it's tackled by Create composition-full/params to add an interface between shared parameter between Composition API and Search API.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I am planning to add a clear interface to avoid code dupplication.
3902472
to
14f3cbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 but with ✅
…[skip ci] Co-authored-by: Clara Muller <[email protected]>
algolia/api-clients-automation#5122 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]>
🧭 What and Why
🎟 JIRA Ticket: Work preparation for https://algolia.atlassian.net/browse/CMP-484
Composition API team is preparing update of the API clients with writes endpoints (see full list of endpoints here).
Goals of this PR is to cleanup / refactor the current Open API spec to ease the integrations of the new endpoints.
Changes included:
➡️ No API client should be updated with this PR, it purely about reorganising code.
Create 3 new directories within
composition-full/schemas
:components
: to group reusable models togetherrequestBodies
: to group and identify request bodies easilyresponse
: to group and identify request response easilyStack PRs
Next steps
composition-full/params
to add an interface between shared parameter between Composition API and Search API.Review tips
PR can be review commit by commit for ease.
🧪 Test
yarn cli build specs all
yarn cli build clients javascript