Skip to content

Conversation

@anton-erofeev
Copy link
Contributor

No description provided.

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.

Thanks for this cleanups @anton-erofeev. I only have one small remark, see my comment

Comment on lines 501 to 500
for (Iterator<? extends TypeMirror> iterator = unusedPermittedSubclasses.iterator();
iterator.hasNext(); ) {
if ( ctx.getTypeUtils().isSameType( iterator.next(), subClassOption.getSource() ) ) {
iterator.remove();
}
}
unusedPermittedSubclasses.removeIf(
typeMirror -> ctx.getTypeUtils().isSameType(typeMirror, subClassOption.getSource()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this change because it means that a new anonymous class has to be created to capture the subClassOption. I prefer not to create new objects for things like this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I reverted this change, thanks

@filiphr
Copy link
Member

filiphr commented Jul 8, 2023

@anton-erofeev seems like there are some formatting issues. Can you have a look at it please?

@anton-erofeev
Copy link
Contributor Author

anton-erofeev commented Jul 19, 2023

@anton-erofeev seems like there are some formatting issues. Can you have a look at it please?

updated but it looks like I can't run checks

@filiphr
Copy link
Member

filiphr commented Jul 29, 2023

You are contributing for the first time @anton-erofeev and that's why the checks don't run automatically. I need to approve them in order for them to run. Just did that, if everything is OK, we'll merge this. For your next contributions this won't be necessary.

@filiphr filiphr added this to the 1.6.0 milestone Aug 1, 2023
@filiphr filiphr merged commit b2dc641 into mapstruct:main Aug 1, 2023
@filiphr
Copy link
Member

filiphr commented Aug 1, 2023

thank @anton-erofeev

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants