RFR 8163798: Add a versionedStream method to JarFile

Alan Bateman Alan.Bateman at oracle.com
Fri Aug 26 12:50:55 UTC 2016


On 26/08/2016 00:30, Steve Drach wrote:

> Hi,
>
> Please review this changeset that adds a versionedStream method to JarFile.
>
> webrev: http://cr.openjdk.java.net/~sdrach/8163798/webrev.00/ <http://cr.openjdk.java.net/~sdrach/8163798/webrev.00/>
> issue: https://bugs.openjdk.java.net/browse/JDK-8163798 <https://bugs.openjdk.java.net/browse/JDK-8163798>
>
I see Peter and others are commenting on the implementation so I'll 
focus on the javadoc for now. In general then I think the javadoc will 
need a bit of work as I don't think it's clear to the reader what is 
returned. It might help to include a few simple examples to show what 
elements will be in the stream. One example might be where the runtime 
version is 10 and there are resources in the base and versions/9 
sections. Also assume I've got resources in versions/10/** that are not 
in other sections then it would be useful to quickly understand if they 
are included are not.

Terminology-wise then I'm not sure if "accessible" should be used, the 
reason is that it will be confused with other types of access, 
particularly access control or even security permissions.

The javadoc also mentions non-public classes and being accessed by 
public classes. I know what you mean but the javadoc shouldn't be 
concerned with that. Clearly the API should be the same on all versions 
but that is something for the introducing in the class description 
rather than this method. Also with modular JARs then you may have public 
types in non-exported packages. I might even have public types in 
non-exported packages in versioned sections that don't override anything 
in the base section.

Another point to include in the javadoc is to make it clear that the 
method behaves the same as stream() then the JAR file is not a 
multi-release JAR.

A minor comment is that there are at least 4 x @{code ...}, it should be 
{@code ...}.

-Alan.


More information about the core-libs-dev mailing list