-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[java][client] fix: Add static modifier to inner class in Java when useSingleRequestParameter=true #20590
base: master
Are you sure you want to change the base?
Conversation
dde8e19
to
77952f4
Compare
77952f4
to
f0016d1
Compare
cc @bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @martin-mfg (2023/08) |
@@ -585,7 +585,7 @@ public ResponseSpec updatePetWithResponseSpec(Pet pet) throws WebClientResponseE | |||
return updatePetRequestCreation(pet); | |||
} | |||
|
|||
public class UpdatePetWithFormRequest { | |||
public static class UpdatePetWithFormRequest { |
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 PR.
It looks like this is a breaking change, right?
If I've the following:
PetApi.UpdatePetWithFormRequest a = new PetApi().new UpdatePetWithFormRequest(12L, "ab", "bc");
I'll get an error after UpdatPetWithFormRequest
becomes static.
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, that's correct, it has to be updated to:
PetApi.UpdatePetWithFormRequest a = new PetApi.UpdatePetWithFormRequest(12L, "ab", "bc");
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.
then i think we need an add an option for fallback as the upcoming v7.12.0 release only allows breaking changes with fallback only.
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.
Sure, any pointers/docs/example for the fallback strategy?
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 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.
actually please put it on hold....
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 think another way is to update the existing option useSingleRequestParameter
to allow static
instead of just true or false so that we don't need to add another option
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 please take a look at updating useSingleRequestParameter
to allow static
?
(when static is provide by the user, we will add "staticRequest" to true in additionalProperties
so that {{#staticRequest}} ... {{/staticRequest}} can be used in the template to add static
in the output
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 dig into that, thanks for the pointers
When generating Java clients with the option
useSingleRequestParameter
set totrue
, there is a inner class generated that is not set as static.@martin-mfg @Zomzog @lwlee2608
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)