-
Notifications
You must be signed in to change notification settings - Fork 759
Add clearer documentation to custom comparers, fill testing holes. #4729
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
Add clearer documentation to custom comparers, fill testing holes. #4729
Conversation
manfred-brands
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only some small issues.
src/NUnitFramework/framework/Constraints/CollectionSupersetConstraint.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/SomeItemsConstraint.cs
Outdated
Show resolved
Hide resolved
|
PR feedback addressed, the only remaining thing is nunit/docs#937 (comment), since its resolution affects this PR. |
manfred-brands
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One rename of Type parameter and one rename of method parameters.
src/NUnitFramework/tests/Constraints/DictionaryContainsValueConstraintTests.cs
Outdated
Show resolved
Hide resolved
src/NUnitFramework/framework/Constraints/DictionaryContainsValueConstraint.cs
Outdated
Show resolved
Hide resolved
manfred-brands
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @RenderMichael. Nothing further.
Fixes #4726
Following the advice of @manfred-brands:
I took the opportunity to find every instance of the in-line comparers to clear things up.
I dropped the
Typeat the end of the the generic parameter names because it began to feel too verbose, I'll add it back in if that's the preferred convention.Some of the more pathological cases had testing holes so I filled those in as well.