-
-
Notifications
You must be signed in to change notification settings - Fork 114
Consider case when both Optional & apiReturnType is used #410 #414
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
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
| ) | ||
| public interface NodeQueryResolver { | ||
|
|
||
| reactor.core.publisher.Mono<java.util.Optional<Node>> node(String id) throws Exception; |
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.
Mono itself can describe 0 or 1 in reactor.Users of the stream should be aware of this.
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, you are right. But have you seen the bug description? There could be some types that can expect Optional as a sub-type. Anyway, this Optional thing can be disabled. That's up to the client to decide.
| } | ||
| } | ||
| if (javaTypeName.startsWith(JAVA_UTIL_LIST) && | ||
| if (computedTypeName.startsWith(JAVA_UTIL_LIST) && |
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 use switch ? I'm implementing kotlin, and there's going to be a lot of conflict.
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.
@jxnu-liguobin before implementing kotlin we need to refactor the code to make it simple for further extensions. I was thinking to do it later next week. So please don't do kotlin yet :)
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.
In fact, the templates and code have been written https://github.com/jxnu-liguobin/graphql-java-codegen/tree/kotlin/src/main/resources/templates/kotlin-lang (In addition to the unit test, the rest are basically ready
), but there is still one problem that has not been solved. I am ready to wait until you refactor.
Fixes #410