Fixed process kotlin coroutines and reactivex types #1014
Conversation
38d75ee to
a9dbafe
Compare
| * | ||
| * @since 4.8.3 | ||
| */ | ||
| public final class ElementUtils { |
There was a problem hiding this comment.
annotate with @Internal
| return element.isNullable() | ||
| || element.getType().isOptional() | ||
| || element.hasStereotype(Nullable.class) | ||
| || element.hasStereotype("javax.annotation.Nullable") | ||
| || element.hasStereotype("jakarta.annotation.Nullable") | ||
| || element.hasStereotype("org.jetbrains.annotations.Nullable") |
There was a problem hiding this comment.
the above checks (apart from isOptional()) are not necessary since isNullable() already checks this.
There was a problem hiding this comment.
@graemerocher Hah! As you see, you're not right ;-)
Reverted
| || element.hasStereotype("androidx.annotation.Nullable") | ||
| || element.hasStereotype("edu.umd.cs.findbugs.annotations.Nullable") | ||
| || element.hasStereotype("org.eclipse.jdt.annotation.Nullable") | ||
| || element.hasStereotype("io.reactivex.annotations.Nullable") | ||
| || element.hasStereotype("io.reactivex.rxjava3.annotations.Nullable") | ||
| || element.hasStereotype("reactor.util.annotation.Nullable") | ||
| || element.hasStereotype("org.jspecify.annotations.Nullable"); |
There was a problem hiding this comment.
hard coding this is not great, these should be replaced by classes like this:
Perhaps in core
There was a problem hiding this comment.
@graemerocher By the way, how is this solution worse than what you suggest? You propose to hardcode the names of annotations separated by classes, I have them in one place, without using additional complex structures and the check goes linearly. In both cases, you will have to hardcode the full path to the annotation class, only in your case I will have to describe 10 more classes without any profit.
In general, I do not understand the better your solution :-)
There was a problem hiding this comment.
because if it is part of core then every module does not have to repeat the same logic and it is encapsulated into a single place. Other parts of the framework will likely need to know what is nullable and that could differ from what OpenAPI believes is nullable if it is only captured here.
There was a problem hiding this comment.
@graemerocher do you suggest to add these annotaions to core module?
There was a problem hiding this comment.
that is what makes sense indeed
There was a problem hiding this comment.
@graemerocher done. Added here: micronaut-projects/micronaut-core#9239
There was a problem hiding this comment.
@graemerocher I will rebase tomorrow in the core. But this commit is for core version 3.9.x, so these changes can only be made in release micronaut-openapi 5.0. Can't you merge these changes?
There was a problem hiding this comment.
@graemerocher could you approve this PR ?
62f72f3 to
73963cb
Compare
|
@graemerocher Hi! Could you approve this PR and this: #988 . After it we can finalize release 4.9.0 |
|
@graemerocher ping |
|
@graemerocher Thanks! Can I publish the release myself or will you do it yourself?? This is draft release: https://github.com/micronaut-projects/micronaut-openapi/releases/tag/untagged-6a3ba8a3c25774f62800 |
|
@graemerocher And about merge to master branch: it's to early. First need merge micronaut 3.9.2 changes to micronaut 4.0.0 (feature to disable slf4j). |
Fixed #1013