Streaming methods #68
Conversation
ctrimble
force-pushed
the
feature_streaming-methods
branch
3 times, most recently
from
ae3d143 to
1098000
Compare
ctrimble
force-pushed
the
feature_streaming-methods
branch
2 times, most recently
from
86946b7 to
50481b5
Compare
|
@wing328 any thoughts on this or other changes like this that improve the interface on JsonNullable? |
|
Thanks for the PR and sorry for late reply,. We're looking for maintainers: #71 Let me know if you're interesting in maintaining this project. Thank you. |
| @@ -0,0 +1,421 @@ | |||
| /* | |||
There was a problem hiding this comment.
I didn’t notice the copyright in any other files (maybe I missed some).
Are you ok with removing this? I don’t exactly know the difference whether you add this or not though. I assume you’re the copyright holder either way. (…I’m not a lawyer though, so I could be completely wrong.)
There was a problem hiding this comment.
I can drop this, if it makes things better for the project.
There was a problem hiding this comment.
Honestly, I just don't know the implications. @wing328 might be able to provide some guidance?
src/main/java/org/openapitools/jackson/nullable/JsonNullable.java
Outdated
Show resolved
Hide resolved
|
I think I have time to get to these changes tonight. |
|
@nrayburn-tech I haven't made the changes yet, but there is now testing that shows exactly where this implementation does not match an equivalent call on Optional. The build will skip comparisons with Optional methods that are not present in the current JDK. Building with Java 17 will show all the issues. The following all need NullPointerExceptions:
I am going to rework testing of ifPresentOrElse to make sure that is consistent as well. |
|
@nrayburn-tech I think I have the changes you asked for completed. Items still open:
I would like to squash this history down and get rebased on master (it looks like the pom.xml cannot merge) before this gets accepted. |
|
After you merge in master, you’ll want to update your pom changes to match the existing format. Use a property for the version. |
|
@nrayburn-tech the project moved to JUnit 5 while this PR was open. I will have to rework the testing annotations. |
ctrimble
force-pushed
the
feature_streaming-methods
branch
from
b2b3080 to
fbafe0f
Compare
|
@nrayburn-tech the tests for this are now reworked and history is squashed down. I still need to deal with this license header and possible indentation issue. |
There was a problem hiding this comment.
I'm not really worried about the license header on the test file or the indent discrepancy. Both are pretty minor right now and can be resolved later in a separate PR if needed. (Tests can be re-written so the copyright doesn't apply, and the code can just be re-formatted if needed.)
found in java.util.Optional. - isUndefined() - orElseGet(Supplier) - orElseThrow() - orElseThrow(Supplier) - ifPresentOrElse(Supplier, Runnable) - filter(Predicate) - map(Function) - flatMap(Function) - or(Supplier) - stream()
ctrimble
force-pushed
the
feature_streaming-methods
branch
from
d7174b2 to
ad0105d
Compare
|
@nrayburn-tech history is squashed. I think this is ready to merge. |
|
@wing328 can you merge this in when you get some time? Thanks. |
|
Just merged. Thanks for the contribution. |
4 participants
This PR adds the streaming methods found on java.util.Optional to the JsonNullable type.
Methods added:
Fixes #7.
Fixes #65.