Cleaner implementation of the equivalency validator #1026
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dennisdoomen
changed the title
dennisdoomen
force-pushed
the
CleanerEquivalencyValidator
branch
from
9ffae61
to
f83920f
Compare
dennisdoomen
changed the title
dennisdoomen
force-pushed
the
CleanerEquivalencyValidator
branch
from
f83920f
to
0c1bee3
Compare
jnyrup
reviewed
Apr 19, 2019
dennisdoomen
force-pushed
the
CleanerEquivalencyValidator
branch
2 times, most recently
from
7bfeff8
to
da42ecc
Compare
jnyrup
approved these changes
Apr 21, 2019
| { | ||
| Key = key; | ||
| Value = value; | ||
| this.reportability = reportability; | ||
| Reportable = reportable; | ||
| RequiresFormatting = requiresFormatting; |
|
|
||
| public bool Reportable => reportability == Reportability.Reportable; | ||
| public bool RequiresFormatting {get; } |
| { | ||
| if (config.AllowInfiniteRecursion || !HasReachedMaximumRecursionDepth(memberAccessPath)) | ||
| if (expectation == null) |
| @@ -1,16 +1,17 @@ | |||
| #region | |||
|
|
|||
| | |||
dennisdoomen
force-pushed
the
CleanerEquivalencyValidator
branch
from
da42ecc
to
386ff2b
Compare
jnyrup
reviewed
May 2, 2019
| @@ -2,6 +2,6 @@ namespace FluentAssertions.Equivalency | |||
| { | |||
| public interface IEquivalencyValidator | |||
| { | |||
| void AssertEqualityUsing(IEquivalencyValidationContext context); | |||
| void RecursivelyAssertEquality(IEquivalencyValidationContext context); | |||
There was a problem hiding this comment.
@dennisdoomen renaming this is a breaking change and should be reverted.
There was a problem hiding this comment.
From the CoreFX Breaking Change Rules
Removing or renaming a member, including a getter or setter from a property or enum members
Though I can't find any code on the internet that implements IEquivalencyValidator except EquivalencyValidator, Every change breaks someone's workflow and last time we thought it didn't matter, it turned out bad.
I think the benefit of renaming a public method versus the risk of introducing a breaking change, does not seem justified.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While preparing a talk about maintainable code, I realized that the
EquivalencyValidatorwas the perfect candidate for refactoring.