-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix(java): escape oneOf naming #268
Conversation
✔️ Deploy Preview for api-clients-automation canceled. 🔨 Explore the source changes: e0294f6 🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/62346119881ee20008337355 |
✗ The generated branch has been deleted. If the PR has been merged, you can check the generated code on the |
d7e9565
to
e0294f6
Compare
@@ -23,3 +23,5 @@ dist | |||
.openapi-generator | |||
|
|||
tests/output/*/.openapi-generator-ignore | |||
|
|||
generators/bin |
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.
What is generating this bin
?
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 generators when built locally without the docker image, I'm not sure why it did not happened before
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 built by the scripts inside the docker image, and should only create a .gradle
and build
folder, do you have something else installed on your IDE ?
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.
Nope, I don't even have a Java extension on my vscode.
Do you want me to remove it from the gitignore?
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.
No if you have it something is generating it, I just wanted to understand what
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'll do more tests in #227 later, will let you know if I know what is the issue
@@ -723,7 +723,10 @@ void deleteByTest0() { | |||
} | |||
|
|||
EchoResponseInterface req = (EchoResponseInterface) assertDoesNotThrow(() -> { | |||
return client.deleteBy(indexName0, SearchParams.of(searchParams0)); |
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 surprised those are the only changes to the test, I think there is something more, I will check.
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 is definitely a missing part with the CTS I think, I still can't make #227 work :(
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-382
Changes included:
In #227 we can see the Java generator fail due to
oneOf
types containing special characters.The
oneOf
names are using the type name, which in the case of an interface, is correct, but not when native types such asList<..>
🧪 Test