#2258 Fixes vague description of @Default and @ConstructorProperties annotations - #2262
Conversation
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
Looks good @nikolas-charalambidis. I have one small remark, but apart from that it looks good to me
| Hence, we say that annotation can be _from any package_. | ||
|
|
||
| For example, MapStruct searches for the annotation _named_ `@ConstructorProperties`. | ||
| As long as there already exists `java.beans.ConstructorProperties` annotation from Java SE, the idea is to reuse such annotation. |
There was a problem hiding this comment.
I would not suggest reusing the java.beans.ConstructorProperties, especially on Java 9+ and the fact that the ConstructorProperties is part of the java.desktop module.
There was a problem hiding this comment.
Thanks for the review. My mind is stuck in Java 8. Well, the idea of this sentence is to give an example of reusing the already existing annotation, not suggesting particularly this one. I remove the sentence for now to avoid the confusion. Do you have an idea of an existing annotation to demonstrate such usage or a better way to rephrase the sentence?
There was a problem hiding this comment.
Missed your question. Unfortunately I don't have an idea
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
As you also added the Lombok part I would suggest adding the fact that users need to add lombok-mapstruct-binding as well starting from Lombok 1.18.16. Otherwise MapStruct and Lombok stops working.
|
@filiphr This is silly mistake of mine as I have forgotten exclude the Lombok part in progress. I have fixed such commit and I will include your note (thank you for the insight!) in my next pull request with the whole Lombok subsection immediately after this issue gets closed and merged into the |
|
Thanks a lot @nikolas-charalambidis, I squashed and merged this PR. Will make sure that it comes in 1.4.2. |
2 participants
This should fix #2258