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

Issue #572 Setup cannot invoke callback with different number of parameters in Moq 4.8.1 #575

Merged
merged 3 commits into from
Jan 17, 2018
Merged

Conversation

Caraul
Copy link
Contributor

@Caraul Caraul commented Jan 17, 2018

Add more parameters to test method as 0 and 1 parameters are special cases for parameterless and extension methods.
Add tests for extension methods with different number of parameters.

Copy link
Contributor

@stakx stakx left a comment

Choose a reason for hiding this comment

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

Looking good! 👍 Besides the request in the one other review comment, could you please also update CHANGELOG.md by inserting the following above ## 4.8.1:

## Unreleased
+
+#### Fixed
+
+* <summary of your change> (@Caraul, #575)

Thanks a lot!

@@ -42,7 +42,7 @@
using System.Globalization;
using System.Linq.Expressions;
using System.Reflection;

using System.Runtime.CompilerServices;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this line a little further up so that using System.* are properly sorted. Also, restore the blank line between using System.* and using Moq.*.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About sorting - Runtime is definitely after Reflection, isn't it?
Blank line is my fault - suspect VS dit that :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Uhhh... yes indeed. You're right, of course. :-)

@stakx stakx merged commit 612c6fa into devlooped:master Jan 17, 2018
@stakx
Copy link
Contributor

stakx commented Jan 17, 2018

Merged. Thanks again for taking the time to submit a fix! 👍

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.

None yet

2 participants