-
Notifications
You must be signed in to change notification settings - Fork 172
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
(#1569) Generify org.cactoos.scalar
package
#1626
(#1569) Generify org.cactoos.scalar
package
#1626
Conversation
org.cactoos.scalar
package
@0crat status |
Codecov Report
@@ Coverage Diff @@
## master #1626 +/- ##
============================================
- Coverage 90.12% 90.10% -0.02%
- Complexity 1602 1603 +1
============================================
Files 298 298
Lines 3745 3749 +4
Branches 122 122
============================================
+ Hits 3375 3378 +3
Misses 337 337
- Partials 33 34 +1
Continue to review full report at Codecov.
|
@0crat status |
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.
@rocket-3 just a few comments. Also, one last check: have you verified that the scalar
package is completely covered with those relaxed generics?
@0crat status |
@victornoel Yes, it's about all the package. If you're going to trust me, I've checked it again yesterday. |
@0crat status |
@rocket-3 I do trust you, my question is just a "protocol" check just to ensure you checked it, not to doubt that you did ^^ that's why we have reviews :) |
@rocket-3 so what about #1628 and the comment I made there: #1628 (comment) ? If the present PR is the one you want to keep, then please close the other |
@victornoel It (#1628) was just the same, I've closed it. |
@rocket-3 ok, cool, thx! |
@rultor merge |
@victornoel OK, I'll try to merge now. You can check the progress of the merge here |
@rocket-3 @victornoel Oops, I failed. You can see the full log here (spent 4min)
|
@victornoel, can't reproduce, cause I don't have ../settings.xml and the warnings doesn't appear without it.
|
@rocket-3 even when running mvn manually with Java 8? I don't believe it's related to the settings.xml file (which is only there for credentials used by rultor). This is a typical problem with varargs and generics in Java, just add suppress warnings for those tests, as in here:
I'm a bit surprised that the CI didn't complain though... but well, let's go with it and add that fix to the corresponding tests :) |
@victornoel Please, try to merge now |
@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 11min) |
@0crat, status |
@0crat status |
For #1569.
Relaxed parameter types by adding wildcards in constructors of classes of
org.cactoos.scalar
package:And
AndInThreads
AndWithIndex
FirstOf
IoChecked
ItemAt
NoNulls
PropertiesOf
Repeated
Retry
ScalarOfSupplier
Solid
Sticky
Synced
Ternary
Unckecked
Xor