feat: Add @AnnotateWith support to mapping methods by chenzijia12300 · Pull Request #2986 · mapstruct/mapstruct
Fixes #2983
Enables @AnnotateWith to support NormalTypeMappingMethod and ValueMappingMethod
I don't really understand why the unit test failed, I shouldn't have affected Issue2897Test.shouldImportNestedClassInMapperImports to run
(⊙﹏⊙)
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
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
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
Thank you very much for your reply 😊 @Zegveld
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your changes @chenzijia12300.
I've left some final comments that we should address before we merge this
Thanks a lot for your changes @chenzijia12300.
I've left some final comments that we should address before we merge this
I apologize for my carelessness and thank you for your comment, I have revised it
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters