-
Notifications
You must be signed in to change notification settings - Fork 164
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
(#1531) Bump cactoos-matchers to 0.24 #1543
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1543 +/- ##
============================================
- Coverage 90.90% 90.85% -0.06%
+ Complexity 1574 1569 -5
============================================
Files 292 292
Lines 3696 3696
Branches 121 121
============================================
- Hits 3360 3358 -2
- Misses 306 308 +2
Partials 30 30
Continue to review full report at Codecov.
|
@0crat status |
@victornoel This is what I know about this job in C63314D6Z, as in §32:
|
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 some few comments below.
@@ -59,7 +58,7 @@ void iteratesListWithIndexFromBiFunc() { | |||
new HasValue<>( | |||
Matchers.allOf( | |||
Matchers.equalTo(true), |
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 We could use here new IsEqual<>(true)
instead of static method Matchers.equalTo
.
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 good point, actually we can even new IsTrue()
directly :) I will do that
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 actually I simplified the code so it's a bit different than proposed
@@ -83,7 +82,7 @@ void iteratesListWithIndexFromBiProc() { | |||
new HasValue<>( | |||
Matchers.allOf( | |||
Matchers.equalTo(true), |
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 Same here.
@@ -65,12 +64,12 @@ public boolean matchesSafely(final List<E> list) { | |||
new Assertion<>( | |||
"must have an index for the sample item", | |||
list.indexOf(this.sample), | |||
new MatcherOf<>(i -> i >= 0) | |||
new Verifies<>(i -> i >= 0) |
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 IMHO, I think having matchers IsGreaterThan
, IsLessThan
and
IsEmpty
for list and iterable in cactoos-matchers will be very nice :) We have only static methods of them (greatThan and lessThan) in Hamcrest.
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 that's a good idea, could you open tickets in cactoos-matchers for those? You will get points for them too! Maybe one for IsGreaterThan
/IsLessThan
and one for IsEmpty
? we can discuss them there then!
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 Great !
19b8a01
to
a23b69d
Compare
@baudoliver7 see my answers and changes, thx |
@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 Done! FYI, the full log is here (took me 8min) |
Job was finished in 19 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 good |
Bump cactoos-matchers to latest version, use
Verifies
instead ofMatcherOf
and replaceJoined
byConcatenated
when possible.