#107 #108
#107 #108
Conversation
|
I will review soon - thanks |
| methodSpec.addStatement("return this").addParameter(parameterSpecBuilder.build()); | ||
| builder.addMethod(methodSpec.build()); | ||
| } | ||
|
|
||
| private void filterOutNotNullAnnotationOnParameter(ParameterSpec.Builder parameterSpecBuilder) { |
There was a problem hiding this comment.
I feel that this portion should be in a different PR. Let's focus this PR solely on the ofNullable. wdyt?
There was a problem hiding this comment.
I agree since the issue discussion has gone off on a tangent.
But the issue this tries to solve is when there's a @NotNull Optional field in record.
In that case current code without the method above would generate
setX(@NotNull X x) {
Optional.ofNullable(x);
}This isn't a problem in my company code base because we never validate builders but it will surprise some.
I've reverted the commit that adds this filter.
There was a problem hiding this comment.
I want to handle the null issue finally and completely which I hope to do with #111 - people seem to have strong opinions on this :D
|
I apologize it's taken me so long to look at this. I left a comment for you to reply to. |
|
Please rebase the commits |
done |
I still see 7 commits. Did you force push your squash? |
|
I'm not sure I follow - my branch has 7 commits that are ahead of the master. I don't see any commits on master that are not present in my branch. |
|
Please squash down to 1 commit. |
removed concreteSetterForOptionalShouldNotHaveNotNullAnnotation Revert "implemented removal of NotNull on concrete setter for Optional" This reverts commit ff3ac3a. implemented removal of NotNull on concrete setter for Optional added test that concrete Optional setter doesnt copy NotNull changed InternalRecordBuilderProcessor to pass shouldAcceptNullForOptionalRawSetter added shouldAcceptNullForOptionalRawSetter test to TestOptional
Fixes #107