Skip to content

#3601 Always use SourceParameterCondition when checking source parameter - #3620

Merged
filiphr merged 7 commits into
mapstruct:mainfrom
filiphr:3601
Jul 20, 2024
Merged

#3601 Always use SourceParameterCondition when checking source parameter#3620
filiphr merged 7 commits into
mapstruct:mainfrom
filiphr:3601

Conversation

@filiphr

@filiphr filiphr commented Jun 2, 2024

Copy link
Copy Markdown
Member

This is a breaking change, with this change whenever a source parameter is used as a source for a target property the condition has to apply to source parameters and not properties

Fixes #3601

filiphr added 2 commits July 6, 2024 09:16
…ce parameter

This is a breaking change, with this change whenever a source parameter is used as a source for a target property the condition has to apply to source parameters and not properties
thunderhook and others added 4 commits July 6, 2024 09:41
These work as expected because the `isNotEmpty` presence checks is having a source parameter passed.
If we change the parameter with the other the method will be invoked again
@thunderhook
thunderhook self-requested a review July 7, 2024 09:48

@thunderhook thunderhook left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few optional "expression" nitpicks in the changelog.

As we already said, we can definitely remove some tests, especially those you mentioned as duplicate scenarios in #3601 (comment)

Nevertheless, i will already approve, the current status could also be merged from my side.

Comment thread NEXT_RELEASE_CHANGELOG.md Outdated
Comment thread NEXT_RELEASE_CHANGELOG.md Outdated
Comment thread NEXT_RELEASE_CHANGELOG.md Outdated
Comment thread NEXT_RELEASE_CHANGELOG.md Outdated
@filiphr

filiphr commented Jul 20, 2024

Copy link
Copy Markdown
Member Author

Thanks @thunderhook. I've applied the proposed changes in the changelog. I also decided to remove all the fixture tests, they were not needed. The main thing that we are testing is already covered by the test that I added. Hope that's OK with you

@filiphr
filiphr merged commit 66f4288 into mapstruct:main Jul 20, 2024
@filiphr
filiphr deleted the 3601 branch July 20, 2024 12:40
@thunderhook

thunderhook commented Jul 20, 2024

Copy link
Copy Markdown
Contributor

That's totally fine, thanks a lot!

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.

Presence check method used only once when multiple source parameters are provided

2 participants