-
Notifications
You must be signed in to change notification settings - Fork 17
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(cts): use custom package to fix openapi-generator-cli #254
Conversation
✔️ Deploy Preview for api-clients-automation canceled. 🔨 Explore the source changes: d3e32ac 🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/6232f8865b2960000996ca3c |
{ text: 'generating requests tests', indent: 4 }, | ||
verbose | ||
).start(); | ||
await run( |
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.
Can you explain to me why we pass this custom generator regardless of gen.language
? IIUC, we should pass for java only, 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.
It generate the same mustache bundle as the ts generator, only with more data on types, so it can be used to generate js tests also, we don't need the duplicate code.
The js tests are generated with this custom generator and it produces no diff.
case 'javascript': | ||
await generateRequestsTests(generator, verbose); |
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.
Are we removing generateRequestsTests
?
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.
They are still generated from the CTS custom generator, written in java but not specific to java
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.
Thanks for the explanation. I missed that part.
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 have the full context around this topic, but it looks good to me. Feel free to get one more approval or not.
✗ The generated branch has been deleted. If the PR has been merged, you can check the generated code on the |
🧭 What and Why
Fixed a bug in openapi-generator-cli and deployed custom package with the fixed waiting for the PR to be merged.
Also removes every custom code used for CTS generation, and exclusively use the Java one (remove the ts one), as they generate the same code now.
Changes included:
🧪 Test
CI