-
Notifications
You must be signed in to change notification settings - Fork 357
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
Support more optionals #4783
Support more optionals #4783
Conversation
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.
this does make sense, but we have to deal with another PR on the same topic which is #4799 thus we will somehow merge those 2 PRs after fixing conflicts which would appear after one or another PR is merged. So, wait with merging but legitimate.
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
@joschi I just merged your changes with mine. Would you mind to review it please?. |
core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/glassfish/jersey/tests/e2e/server/OptionalParamConverterNoProviderTest.java
Show resolved
Hide resolved
core-common/src/main/java/org/glassfish/jersey/internal/inject/ParamConverters.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
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.
LGTM. 👍
Adding support for OptionalInt, OptionalDouble and OptionalLong.
This was mentioned by @joschi here: #4651