-
-
Notifications
You must be signed in to change notification settings - Fork 1k
#3119 Add qualifiedBy and qualifiedByName to SubclassMapping annotation #3120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
500433a to
76c37a4
Compare
|
As pointed out in #3119 (comment) using the qualifiers present in the BeanMapping annotation wasn't the right thing to do, so i expanded the SubclassMapping annotation with qualifiedBy and qualifiedByName. |
filiphr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great @EvaristeGalois11. Thanks a lot for the PR and sorry that it took so long to merge it.
I've left one comment, but I'll still merge the PR without it. I'll create an issue for the improvement of the error line location
| diagnostics = @Diagnostic( | ||
| type = ErroneousSubclassQualifiedByMapper.class, | ||
| kind = javax.tools.Diagnostic.Kind.ERROR, | ||
| line = 15, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this should be the line with the @SubclassMapping, i.e. line 13
| diagnostics = @Diagnostic( | ||
| type = ErroneousSubclassQualifiedByNameMapper.class, | ||
| kind = javax.tools.Diagnostic.Kind.ERROR, | ||
| line = 15, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this should be the line with the @SubclassMapping, i.e. line 13
|
@EvaristeGalois11 I've created #3202 for the line location. In case you are interested in adding that, you have the first dibs on the issue :). |
Thank you instead for accepting the PR, I had a really great time working on this project and i plan to contribute a bit more if i can in the future! And no need to apologize about the wait, we all have personal stuff to take care :D |
I am happy to hear that it was fun for you to work on this project. We appreciate any help we can get. So I won't say no for any help :) |
Close #3119
It's a simple fix that just passes along the qualifiers present on the BeanMapping annotation to aid the resolution of the subclass specific method.
I don't know if this is the right approach or you would prefer two specific new attributes on the SubclassMapping annotation. For my needs it is enough.