-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
[Fix] Fixed null in Parameter and added functionality Binary And and Or with different Types #86
Conversation
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 84.07% 84.19% +0.11%
==========================================
Files 25 25
Lines 3090 3131 +41
Branches 449 471 +22
==========================================
+ Hits 2598 2636 +38
+ Misses 364 357 -7
- Partials 128 138 +10
Continue to review full report at Codecov.
|
Maybe this could also be included in the nuget ;-) ? |
Can this be merged? |
I provided some code-review comments, can you please fix ? |
Where? I don't see them! |
} | ||
|
||
[Fact] | ||
public void ExpressionTests_Test_BinaryAndOr() |
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.
Some remarks:
- Does this method also test
OR
? Because I only see&
? - Rename this method to
ExpressionTests_BinaryAnd
and alsoExpressionTests_BinaryOr
? - Move this method to the correct location (I try to keep a A-Z sorting applied to this test class to keep things organized)
- Also check the result (result0, result1, ...) using Assert, else the unit-test is not complete.
Review is requested now, sorry I did not press the button... ;-) |
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.
done
if it's okay, can you merge and create a nuget? |
1.0.7.2 / 1.0.4.2 added to NuGet |
No description provided.