Skip to content
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

feat(specs): add recommend spec and client #19

Merged
merged 7 commits into from
Nov 30, 2021
Merged

Conversation

shortcuts
Copy link
Member

Summary

  • Add recommend client and spec
  • Restructure output to make it look closer to the algoliasearch-client-javascript repository
  • Update playground and README
  • Add 402 error when feature is not available
  • Small mustache update to support package name during generation

Disclaimer

Types

Same as #17, "complex" types are weirdly generated.

If we decide to go with a more optimal yml (avoid rewriting same parameters, like I tried on both PRs), some types are exported/not used etc. but when we duplicate the content in the yml, the output is better.

Ref

openapi-generator struggle with relative paths, which requires us to rewrite some logic like: https://github.com/algolia/api-client-automation-experiment/blob/429b1ba524d4a44182402eda26e7d66332fca03d/specs/recommend/paths/getRecommendations.yml#L35-L42

(and also create useless types).

Next

  • I'll try with a less optimal yml file to see if the output is better
  • We have a lot of commands in our package.json, I guess it's a good time to focus on scripts :D

@shortcuts shortcuts marked this pull request as ready for review November 29, 2021 11:47
@shortcuts shortcuts requested review from damcou and millotp and removed request for damcou November 29, 2021 11:47
@shortcuts
Copy link
Member Author

shortcuts commented Nov 29, 2021

✔️ Code generated!

Name Link
🔨 Triggered by f9321efd5b282c971b82204995d5174a78702d35
🔍 Generated code 328baeebc59a4c8e95e7f269629441050869f434
🌲 Generated branch generated/feat/java-retry

@shortcuts shortcuts self-assigned this Nov 29, 2021
"generate": "yarn generate:js && yarn lint",
"generate:search": "yarn generate:js:search && yarn lint",
"playground:js": "yarn workspace algoliasearch-client-javascript-playground start",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we keep a playground:js command which launch every tests ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it specific to the client but if you want them grouped I don't really mind: https://github.com/algolia/api-client-automation-experiment/blob/459e6288f2784f55ce73013fb696d134e5c365c7/package.json#L27-L28

Copy link
Contributor

@damcou damcou Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that, but it could be useful to have both (with a command which launch playground:js:* )

damcou
damcou previously approved these changes Nov 29, 2021
Copy link
Contributor

@damcou damcou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now :) 🚀

@shortcuts
Copy link
Member Author

Unused import in this PR will be removed once eslint is added, there was too much changes to keep it in this PR

millotp
millotp previously approved these changes Nov 30, 2021
Copy link
Collaborator

@millotp millotp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this creates way to much duplicate code, we could centralize the models and utils, but great job nonetheless !

"utils:import-js": "mkdir -p -- clients/algoliasearch-client-javascript/utils && cp -R clients/utils/javascript/ clients/algoliasearch-client-javascript/utils"
"playground:js:search": "yarn workspace javascript-playground start:search",
"playground:js:recommend": "yarn workspace javascript-playground start:recommend",
"utils:import-js": "mkdir -p -- clients/algoliasearch-client-javascript/${CLIENT}/utils && cp -R clients/algoliasearch-client-javascript/utils/ clients/algoliasearch-client-javascript/${CLIENT}/utils"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go on the same package style as the original client, instead of copying multiples times utils I think we should make a package-utils, wdyt ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely, we still need to improve the tooling on this side

Base automatically changed from feat/settings-spec to main November 30, 2021 08:52
@shortcuts shortcuts dismissed stale reviews from millotp and damcou via 91d0b5d November 30, 2021 08:52
@shortcuts shortcuts merged commit ef603fb into main Nov 30, 2021
@shortcuts shortcuts deleted the feat/recommend-spec branch November 30, 2021 09:58
shortcuts added a commit that referenced this pull request Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants