feat: Add @AnnotateWith support to mapping methods#2986
feat: Add @AnnotateWith support to mapping methods#2986filiphr merged 5 commits intomapstruct:mainfrom
Conversation
|
I don't really understand why the unit test failed, I shouldn't have affected Issue2897Test.shouldImportNestedClassInMapperImports to run |
Looking into this issue. Might be that something in the main branch is broken on that part. Seems to be related to how mapstruct handles automated imports and user defined imports. Edit: created issue #2990 for this, fix already existed under #2984 |
|
It was my fault. Everything should be fixed now on main. I restarted the jobs |
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
The implementation looks good @chenzijia12300. I've left some comments for some small fixes there.
In addition to that, I've left some comments for the tests. Can we please move them a bit like I proposed?
processor/src/main/java/org/mapstruct/ap/internal/model/AbstractMappingMethodBuilder.java
Outdated
Show resolved
Hide resolved
processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java
Outdated
Show resolved
Hide resolved
processor/src/test/java/org/mapstruct/ap/test/bugs/_2983/Issue2983Mapper.java
Outdated
Show resolved
Hide resolved
processor/src/test/java/org/mapstruct/ap/test/bugs/_2983/Issue2983Mapper.java
Outdated
Show resolved
Hide resolved
|
Regarding the failing tests. I tried running the builds, but it seems like it doesn't really do a proper merge commit or something like that. Can you please rebase on top of main and push to your branch so that you get the latest changes? |
|
Thank you very much for your review @filiphr , I will try to implement your proposal at a later time |
Thank you very much for your reply 😊 @Zegveld |
…ckstyle for StreamMappingMethod constructor methods
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
Thanks a lot for your changes @chenzijia12300.
I've left some final comments that we should address before we merge this
processor/src/main/java/org/mapstruct/ap/internal/model/StreamMappingMethod.java
Show resolved
Hide resolved
processor/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateBeanMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
...or/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateIterableMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
processor/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateMapMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
...ssor/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateStreamMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
...essor/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateValueMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
...essor/src/test/java/org/mapstruct/ap/test/annotatewith/AnnotateValueMappingMethodMapper.java
Outdated
Show resolved
Hide resolved
I apologize for my carelessness and thank you for your comment, I have revised it |
Thanks a lot for your revisions. Everything is looking great now. I've merged this to main now. And no need to apologise, that's why we have PRs, so we can share knowledge and ideas. |
|
Thank you |
3 participants
Fixes #2983
Enables
@AnnotateWithto supportNormalTypeMappingMethodandValueMappingMethod