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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(specs): bundle specs #91

Merged
merged 18 commits into from
Jan 18, 2022
Merged

feat(specs): bundle specs #91

merged 18 commits into from
Jan 18, 2022

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Jan 14, 2022

馃Л What and Why

馃師 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-256

Changes included:

We previously had issue with our specs due to path being too complex/deep. This PR introduces a new tool to bundle/lint specs properly.

  • Add summary to specs.
  • Fix wrong naming for builtInOperations.
  • Fix "in between" refs workaround.
  • Update specs bundling script
  • Update client gen script

馃И Test

CI :D

@shortcuts shortcuts requested review from a team and damcou and removed request for a team January 17, 2022 10:56
@shortcuts shortcuts self-assigned this Jan 17, 2022
@shortcuts shortcuts marked this pull request as ready for review January 17, 2022 11:12
@shortcuts shortcuts marked this pull request as draft January 17, 2022 11:25
@shortcuts shortcuts marked this pull request as ready for review January 17, 2022 13:47
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.

So many fixes ! I love good tools

.github/workflows/check.yml Show resolved Hide resolved
scripts/builds/specs.sh Outdated Show resolved Hide resolved
specs/recommend/common/schemas/SearchParams.yml Outdated Show resolved Hide resolved
Comment on lines +20 to +21
'404':
$ref: ../../../common/responses/IndexNotFound.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this error makes sense, as there is not indexName in the request (same thing for others endpoint where you added 404)

Copy link
Member Author

Choose a reason for hiding this comment

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

It was for the sake of making the linter pass, but as errors does not work yet, I believe we should have a proper task to fix all of those issue

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes we will definitely forget otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created https://algolia.atlassian.net/browse/APIC-265 with a high priority for the next sprint

millotp
millotp previously approved these changes Jan 18, 2022
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.

nice !

@shortcuts shortcuts merged commit 622bce1 into main Jan 18, 2022
@shortcuts shortcuts deleted the feat/APIC-256/specs-bundle branch January 18, 2022 13:09
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

2 participants