-
Notifications
You must be signed in to change notification settings - Fork 26
refactor(specs): mutualise code between Search API & Comp API for search query parameters #5125
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.
|
825c35d
to
aec0c49
Compare
5fd8e66
to
9f7905b
Compare
advancedSyntaxFeaturesEnum: | ||
type: string | ||
enum: [exactPhrase, excludeWords] | ||
x-categories: | ||
- Query strategy |
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.
Yes but what should be the good values ? The previous enum was using the name of the query parameter.
If I call the enum by the query parameter name, what should be the value to identify the query parameter ?
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 generator only create a model for the enum, not the array of enum, so I think you can just name it AlternativesAsExact
like before, no?
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.
By putting the same values I have the following error:
/Users/clara.muller/workspace/repos-algolia/api-clients-automation/specs/common/schemas/IndexSettings.yml
896:0 error Parsing error: Map keys must be unique
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.
I made a proposal in the last commit, tell me what do you think
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.
yarn cli generate javascript
is working locally
79cac42
to
ef18a2f
Compare
@Fluf22 @shortcuts Small ping on the PR. |
ef18a2f
to
3914261
Compare
4cefdd3
to
93b68b4
Compare
@Fluf22 @shortcuts 👋 Clara handed this PR off to me before she left for PTO. I pushed a commit to fix the issue with her last commit, and the CI is green again. Could you please have another look? It seems like there was an unresolved comment: #5125 (comment) |
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.
that's a great cleanup!
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.
i'm not sure what's the point of this file anymore if all it does is referencing the common specification, we could directly import the common ones instead of this one, no?
640a997
to
c004dd2
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.
clean! gg
…rch query parameters (#5125) (generated) [skip ci] Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[email protected]>
…rch query parameters (generated) algolia/api-clients-automation#5125 Co-authored-by: algolia-bot <[email protected]> Co-authored-by: Clara Muller <[email protected]> Co-authored-by: shortcuts <[email protected]> Co-authored-by: Gavin Wade <[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.
Stack PRs:
Changes included:
Remove code duplication in file
specs/composition-full/common/params/Search.yml
where a lot of parameters where copy pasted from the filesspecs/common/schemas/SearchParams.yml
&specs/common/schemas/IndexSettings.yml
.To achieve that, multiple schema has been extracted to make them available to be re-use directly.
Schemas have been extracted from the following objects:
baseSearchParamsWithoutQuery
inspecs/common/schemas/SearchParams.yml
indexSettingsAsSearchParams
inspecs/common/schemas/IndexSettings.yml
🧪 Test
yarn cli build specs all
yarn cli build clients javascript
yarn cli generate javascript
yarn cli generate go