Skip to content
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

[6.0.1] Allow non-nullable value comparer to be applied on nullable properties #26430

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

AndriySvyryd
Copy link
Member

Fixes #26429

Description

An exception is thrown when configuring nullable properties with a non-nullable value comparer via pre-convention model configuration.

Customer impact

The only workaround is to avoid using pre-convention model configuration and configure each property explicitly.

How found

Customer report on RC2.

Regression

Yes, from RC1 in #25969. However, this is a new feature in 6.0

Testing

Test for this scenario added in the PR. Tests for value comparers with different nullability are already present, but don't use model building.

Risk

Low; the fix is to not throw an exception for a specific case.

@ajcvickers ajcvickers added this to the 6.0.x milestone Oct 22, 2021
@AndriySvyryd
Copy link
Member Author

Not adding a quirk mode as that would just result in an exception being thrown.

@leecow leecow modified the milestones: 6.0.x, 6.0.1 Nov 2, 2021
@AndriySvyryd AndriySvyryd changed the title [6.0.1] Allow non-nullable value comparer to be applied on nullable properties Allow non-nullable value comparer to be applied on nullable properties Nov 9, 2021
@AndriySvyryd AndriySvyryd merged commit 539aee7 into release/6.0 Nov 9, 2021
@AndriySvyryd AndriySvyryd deleted the Issue26429 branch November 9, 2021 21:11
@ajcvickers ajcvickers removed this from the 6.0.1 milestone Nov 17, 2021
@ajcvickers ajcvickers changed the title Allow non-nullable value comparer to be applied on nullable properties [6.0.1] Allow non-nullable value comparer to be applied on nullable properties Nov 17, 2021
This pull request was closed.
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.

3 participants