Repository navigation
Feat/allow custom comparer null self - #5218
Conversation
c6eef57 to
d65e9e2
Compare
manfred-brands
left a comment
There was a problem hiding this comment.
@mikkelbu you beat me to it.
The reorder in NUnitEqualityComparer are good.
I''m not sure if all generic method needs to be nullable. For generics that have no constraint it should depend on the caller.
If a user adds a compare for int should that imply null?
Or only if the user explicitly adds a comparer for int?.
Note I have only reviewed on my phone.
|
@manfred-brands I was actually in the process of writing a comment on the issue about not liking that this changed so much public API :) - #3692 (comment) - I'll try to see how much I can remove |
57dcb0d to
0c88662
Compare
There was a problem hiding this comment.
I think I'll add some more tests of this
|
@mikkelbu Is this WIP ? Can we add it to 5 or should it wait for 6. |
|
@mikkelbu The best way of getting rid of the merge conflicts, which is caused by the release/5.X branch being too far back in time, is to create this branch again, and cherry pick the commits that are actually yours. |
0c88662 to
9c18812
Compare
|
I have reapplied the commit to main. |
|
I noticed that some cases are not covered by the general changes. I'll address those shortly. |
Thanks for taking over on this @manfred-brands 💪. For the time being I can only find very little time, so I doubt that I would have looked at this |
|
Moved the user comparer before the internal Updated the tests to cover all 6 This is now ready for review . |
Fixes #3692