Skip to content

Fix issue #2449: SubClass annotations are missing from the base class. #2467

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

Closed
wants to merge 1 commit into from

Conversation

yaelah
Copy link

@yaelah yaelah commented Mar 29, 2016

Added a children property to CodegenModel and write out the necessary annotations.
Please give me your feedback on this change and let me know if there is something else I should do.
thanks.

@wing328
Copy link
Contributor

wing328 commented Mar 30, 2016

@yaelah thanks for the PR. As shown in the commit tab, the commit is not linked to your Github account, which means your credit won't count in https://github.com/swagger-api/swagger-codegen/graphs/contributors

I would suggest you to fix it by setting a proper git email address in your local environment.

@wing328 wing328 added this to the v2.2.0 milestone Mar 31, 2016
@yaelah
Copy link
Author

yaelah commented Apr 1, 2016

I am not sure how to get rid of the first 2 commits. Should I just delete this pull request and create a new fork with the correct account?
thanks.

@wing328
Copy link
Contributor

wing328 commented Apr 9, 2016

@yaelah yaelah force-pushed the master branch 2 times, most recently from e867458 to 5ca261e Compare April 9, 2016 17:28
… base class.

Added a children property to CodegenModel and write out the necessary annotations.
@@ -47,6 +47,7 @@

public JavaClientCodegen() {
super();
supportsInheritance = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot be done at the moment. See : #1120 (comment)
Does it work if you keep supportsInheritance to false ?

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, I have a PR on the swagger-parser that would make supportsInheritance work : swagger-api/swagger-parser#246

@yaelah
Copy link
Author

yaelah commented Jun 29, 2016

This is a matter of taste :)
Open source projects such as WebKit and Chromium prefer early return over indentation.

@@ -123,6 +123,19 @@ public void processOpts() {
}
}
}
// Let parent know about all its children
for (String name : allModels.keySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

for (CodegenModel cm : allModels.values()) ?

@cbornet
Copy link
Contributor

cbornet commented Jun 29, 2016

I'm OK with early return if there is code after the return statement which is not the case here. But you're right, that's opiniated...

@wing328 wing328 modified the milestones: v2.3.0, v2.2.0 Jul 9, 2016
@wing328 wing328 modified the milestones: v2.2.1, v2.3.0 Aug 8, 2016
@wing328 wing328 modified the milestones: v2.3.0, v2.4.0 Dec 23, 2017
@wing328 wing328 closed this Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants