Conversation
|
Looking good! I also started doing this and my coverage for Other suggestions:
Otherwise this is just fine. As @enebo was saying to me, there will still be cases where we regress, but we could set up a JDK9 build + JDK8 test run in CI to help avoid that. |
headius
left a comment
headius
left a comment
There was a problem hiding this comment.
Looking good. Make sure all appropriate call sites get covered by using IDE tooling to find those call sites.
| import org.jruby.util.StringSupport; | ||
| import org.jruby.util.io.EncodingUtils; | ||
| import org.jruby.util.unsafe.UnsafeHolder; | ||
| import static org.jruby.util.io.ChannelHelper.*; |
There was a problem hiding this comment.
Expand this and the others; we generally don't use * unless it's more than like 10 imports from the same place.
| **/ | ||
| public static Buffer clearBuffer(Buffer buffer) { | ||
| return buffer.clear(); | ||
| } |
There was a problem hiding this comment.
This and the others can be generified like so:
public static <T extends Buffer> T clearBuffer(T buffer) {
return (T) buffer.clear();
}This allows methods with variadic returns to still be called and return the right type of thing (e.g. you can call ByteBuffer x = clearBuffer(byteBuffer)).
| } | ||
|
|
||
| /** | ||
| * Java 9 introduced overridden methods |
There was a problem hiding this comment.
First complete sentence of javadoc (with period, so it generates right) should say what this method does. The explanation comes second.
Fixes jruby#5451. Thanks to @ahorek for the original patch (jruby#5459).
3 participants
#5451