-
Notifications
You must be signed in to change notification settings - Fork 21
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
(#20) Cleanup matcher #227
Conversation
Codecov Report
@@ Coverage Diff @@
## master #227 +/- ##
============================================
+ Coverage 98.83% 98.86% +0.02%
- Complexity 142 147 +5
============================================
Files 28 28
Lines 343 352 +9
Branches 4 7 +3
============================================
+ Hits 339 348 +9
Misses 4 4
Continue to review full report at Codecov.
|
a940952
to
aabcfa7
Compare
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.
@victornoel Please, see comments below.
@@ -46,7 +47,7 @@ | |||
* @since 0.24 | |||
* @checkstyle JavadocMethodCheck (500 lines) | |||
*/ | |||
public final class RunsInThreads<T> extends TypeSafeMatcher<Func<T, Boolean>> { | |||
public final class RunsInThreads<T> extends TypeSafeDiagnosingMatcher<Func<? super T, Boolean>> { |
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.
@victornoel I think it isn't necessary to use ? super T
in TypeSafeDiagnosingMatcher<Func<? super T, Boolean>>
. Just T
would be enough.
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.
@baudoliver7 yep, I suppose it's not really needed here, I will remove it to not make things over complex
@@ -58,41 +54,15 @@ void matches() { | |||
* Gives negative testing result for the invalid arguments. | |||
*/ | |||
@Test | |||
void matchStatus() { | |||
void mismatches() { |
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.
@victornoel What about the match case.
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.
@baudoliver7 I will suppose you mean testing Matches
using Matches
? it's a good idea
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.
@victornoel Yes
aabcfa7
to
fd1071e
Compare
@baudoliver7 thank you, it's done |
@victornoel thx, looks good to me :) |
@rultor merge |
@victornoel OK, I'll try to merge now. You can check the progress of the merge here |
@victornoel Oops, I failed. You can see the full log here (spent 4min)
|
fd1071e
to
9e2eb26
Compare
@rultor merge |
@victornoel OK, I'll try to merge now. You can check the progress of the merge here |
@victornoel Oops, I failed. You can see the full log here (spent 9min)
|
9e2eb26
to
ce70d22
Compare
@rultor merge |
@victornoel OK, I'll try to merge now. You can check the progress of the merge here |
@victornoel Done! FYI, the full log is here (took me 10min) |
Job was finished in 24 hours, bonus for fast delivery is possible (see §36) |
@sereshqua/z please review this job completed by @baudoliver7/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
@0crat quality acceptable |
@baudoliver7 please make sure you will find at least 3 issues during next CR, thanks |
This is for #20 that was incorrectly closed.