-
Notifications
You must be signed in to change notification settings - Fork 165
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
(#996) Make ListEnvelope public and consolidate API #1096
Conversation
Job #1096 is now in scope, role is |
Codecov Report
@@ Coverage Diff @@
## master #1096 +/- ##
=========================================
Coverage 88.77% 88.77%
+ Complexity 1588 1587 -1
=========================================
Files 275 275
Lines 3947 3947
Branches 214 214
=========================================
Hits 3504 3504
Misses 390 390
Partials 53 53
Continue to review full report at Codecov.
|
f0f253f
to
f428eaf
Compare
This pull request #1096 is assigned to @vzurauskas/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @llorllale/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be no monetary reward for this job |
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 Review done.
} | ||
|
||
@Override | ||
public String toString() { | ||
public final String toString() { |
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.
Consider removing this toString
entirely. I think it should be final in parent abstract class, they both decorate the same List
object anyway.
} | ||
|
||
@Test(expected = UnsupportedOperationException.class) | ||
public void unsupportedRemove() { |
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.
removeIsNotSupported
, to be consistent with naming of other tests?
} | ||
|
||
@Test(expected = UnsupportedOperationException.class) | ||
public void unsupportedSet() { |
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.
setIsNotSupported
, to be consistent with naming of other tests?
} | ||
|
||
@Test(expected = UnsupportedOperationException.class) | ||
public void unsupportedAdd() { |
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.
addIsNotSupported
, to be consistent with naming of other tests?
f428eaf
to
f71ae47
Compare
@vzurauskas thanks, I've applied your recommendations and also added a |
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.
@llorllale Looks good.
Guys, when you merge it, can you make a new release with this fix? |
@rultor merge |
@llorllale OK, I'll try to merge now. You can check the progress of the merge here |
@llorllale Done! FYI, the full log is here (took me 12min) |
@sereshqua/z please review this job completed by @vzurauskas/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed |
The job #1096 is now out of scope |
@rultor release, tag is |
@llorllale OK, I will release it now. Please check the progress here |
Payment to |
@llorllale Done! FYI, the full log is here (took me 14min) |
@sereshqua I saw your comment when it appeared. No need to comment about past comment ;) |
@piotrkot you saw, but you haven't replied :) |
@0crat quality acceptable |
Quality review completed: +4 point(s) just awarded to @sereshqua/z |
Order was finished, quality is "acceptable": +15 point(s) just awarded to @vzurauskas/z |
@sereshqua I didn't answer as it wasn't a question ;) |
This is for #996:
ListEnvelope
public and clarified doc on its behavioursubList
was returning aList
with the same behaviour (note that it means sublist is now called lazily)toString
and simplifiedMapped
as a result