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