Hi Brent / Daniel, > On Nov 9, 2018, at 4:41 PM, Brent Christian <brent.christ...@oracle.com> > wrote: > > On 11/9/18 2:04 PM, Brian Burkhalter wrote: >> An updated patch is at >> http://cr.openjdk.java.net/~bpb/6516099/webrev.06/ >> <http://cr.openjdk.java.net/~bpb/6516099/webrev.06/> >> <http://cr.openjdk.java.net/~bpb/6516099/webrev.06/ >> <http://cr.openjdk.java.net/~bpb/6516099/webrev.06/>> >> including a revision of the implementation to align with the words. The >> tests are not updated yet. > > I think this looks quite good. > > My only comment is that > > 568 * If {@code n} is zero or negative, then no bytes are skipped. > > seems a bit redundant, given the opening paragraph. I could take it or leave > it.
If the content of the @implSpec is supposed to be self-contained, then I think it should remain. As that does not appear to be the case, I agree that it could be deleted. > On Nov 12, 2018, at 2:48 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > But I wonder if skip(n) returning a negative value > (ns < 0) when n has been asserted to be > 0 should be > considered as an error, and if an IOException should > be thrown? I tend to agree that covering this sort of aberration is probably a good idea. I have updated the patch per the foregoing comments and have also improved the Skip test: http://cr.openjdk.java.net/~bpb/6516099/webrev.07/ Thanks, Brian
