-
Notifications
You must be signed in to change notification settings - Fork 21
feat(specs): update try transformation specs for no-code #4974
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): update try transformation specs for no-code #4974
Conversation
✔️ Code generated!
📊 Benchmark resultsBenchmarks performed on the method using a mock server, the results might not reflect the real-world performance.
|
0b7a400
to
437cf0c
Compare
- type | ||
- input |
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 don't think we should require them here
We could create another object where it's required, but here, it would be a bc I 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.
Sorry, I didn't get it.
The code
field was previously required; now, instead of the code
field, we want to use type
and input
. So that is why I made them required, because the endpoint wouldn't work without them.
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 if people use this prop in their code, it will not work anymore.
For example, if you have this in your JS code:
const transformation: Transformation = { code: '' }
Typescript will be yelling at you because you don't implement type
and input
, when you'll upgrade to the latest version
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.
it still works with the legacy payload right ? @Fluf22 is right, this is a breaking change if you put them as required
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 sent my message without seeing your second one thomas
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.
Okay. I didnt aware of this will effect and break the generated code. Btw, this is also the case for create payload so I need to revert that change.
But what is the recommendation in here? Make nothing required?
(The api supports legacy payload and the new payload at the same time. So there is no breaking change in the actual 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.
@millotp I'd suggest creating a second payload and using a oneOf?
But maybe this would be bc in some languages?
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.
Only other option is to make nothing required, yes
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.
having a oneOf is breaking too, I would suggest making everything optional for now, and in a month or so, make the new payload required and leave code
as optional.
We don't have a lot of way to deal with breaking changes because of the SLA...
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.
updated with fa35a7b
…erated) [skip ci] Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
algolia/api-clients-automation#4974 Co-authored-by: algolia-bot <accounts+algolia-api-client-bot@algolia.com> Co-authored-by: Mehmet Ali Gok <33124154+mehmetaligok@users.noreply.github.com>
🧭 What and Why
A follow-up to #4901.
This PR adds no-code transformations support to the try endpoint specs.
Changes included: