Skip to content

Conversation

@EvaristeGalois11
Copy link
Contributor

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.

@EvaristeGalois11 EvaristeGalois11 marked this pull request as draft December 24, 2022 18:04
@EvaristeGalois11 EvaristeGalois11 changed the title #3119 Pass qualifiers to subclass mapping resolving attempt #3119 Add qualifiedBy and qualifiedByName to SubclassMapping annotation Dec 25, 2022
@EvaristeGalois11 EvaristeGalois11 marked this pull request as ready for review December 25, 2022 13:39
@EvaristeGalois11
Copy link
Contributor Author

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.

Copy link
Member

@filiphr filiphr left a 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,
Copy link
Member

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,
Copy link
Member

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

@filiphr
Copy link
Member

filiphr commented Mar 17, 2023

@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 :).

@EvaristeGalois11 EvaristeGalois11 deleted the issue/3119 branch March 17, 2023 21:37
@EvaristeGalois11
Copy link
Contributor Author

This looks great @EvaristeGalois11. Thanks a lot for the PR and sorry that it took so long to merge it.

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

@filiphr
Copy link
Member

filiphr commented Apr 16, 2023

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!

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support qualifier for SubclassMapping annotated methods

2 participants