Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-13 Thread Mandy Chung
> On Sep 12, 2016, at 5:14 PM, Steve Drach wrote: > > http://cr.openjdk.java.net/~sdrach/8163798/webrev.06/ > +1 Mandy

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-13 Thread Paul Sandoz
> On 12 Sep 2016, at 17:14, Steve Drach wrote: > > I guess I’m going to keep doing this until I get it right ;-) Here’s another > webrev that doesn’t use an exception for a common case, and addresses most of > Mandy concerns. > > http://cr.openjdk.java.net/~sdrach/8163798/webrev.06/ >

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Steve Drach
I guess I’m going to keep doing this until I get it right ;-) Here’s another webrev that doesn’t use an exception for a common case, and addresses most of Mandy concerns. http://cr.openjdk.java.net/~sdrach/8163798/webrev.06/ Comments i

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Mandy Chung
> On Sep 12, 2016, at 2:17 PM, Steve Drach wrote: > > Here’s a new webrev addressing Paul’s additional concerns > > http://cr.openjdk.java.net/~sdrach/8163798/webrev.04/ > This version looks good. Can you add the javadoc to describe wha

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Steve Drach
Here’s a new webrev addressing Paul’s additional concerns http://cr.openjdk.java.net/~sdrach/8163798/webrev.04/ > Ok, better. Now there is no need to create an instance of VersionedStream, > all methods can be static. Done > > I wonder

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Paul Sandoz
> On 12 Sep 2016, at 12:36, Steve Drach wrote: > >>> I made a simple change, the new webrev is >>> http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ >>> >>> >> >> I don’t like the state interplay between allowedVersions and getBaseS

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Steve Drach
Here’s a new webrev that addresses Claes’ and Paul’s concerns http://cr.openjdk.java.net/~sdrach/8163798/webrev.03/ > On Sep 11, 2016, at 1:12 PM, Steve Drach wrote: > > I made a simple change, the new webrev is > http://cr.openjdk.java.

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Steve Drach
>> I made a simple change, the new webrev is >> http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ >> >> > > I don’t like the state interplay between allowedVersions and getBaseSuffix, > and the filtering for null. Consider merging fil

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Steve Drach
> jdk/internal/util/jar/VersionedStream.java: > > - use Integer.parseInt(name, vptr, ptr, 10) to avoid creating vstr on > every entry done > > - folding allowedVersions into getBaseSuffix seems easier than to keep > global state (sptr) and splitting the logic over a filter and a map > operat

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-12 Thread Paul Sandoz
> On 11 Sep 2016, at 13:12, Steve Drach wrote: > > I made a simple change, the new webrev is > http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ > > I don’t like the state interplay between allowedVersions and getBaseSuffix, and th

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-11 Thread Claes Redestad
Hi, jdk/internal/util/jar/VersionedStream.java: - use Integer.parseInt(name, vptr, ptr, 10) to avoid creating vstr on every entry - folding allowedVersions into getBaseSuffix seems easier than to keep global state (sptr) and splitting the logic over a filter and a map operation - distinc

Re: RFR: 8163798: Create a JarFile versionedStream method

2016-09-11 Thread Steve Drach
I made a simple change, the new webrev is http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ > On Sep 9, 2016, at 4:02 PM, Steve Drach wrote: > > Hi, > > Please review this changeset that adds a VersionedStream class to the > jdk.int

RFR: 8163798: Create a JarFile versionedStream method

2016-09-09 Thread Steve Drach
Hi, Please review this changeset that adds a VersionedStream class to the jdk.internal.util.jar package. Some may recall that I submitted a similar RFR a few weeks ago; this is a redesign from that one. We decided not to make a public JarFile::versionedStream method at this time. Once we get