On Sun, 18 Sep 2022 19:49:51 GMT, Alan Bateman <[email protected]> wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Incorporated latest round of input
>
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 36:
>
>> 34: * The {@code JarInputStream} class, which extends {@link
>> ZipInputStream},
>> 35: * is used to read the contents of a JAR file from an input stream.
>> 36: * It provides support for reading an optional {@link
>> JarFile#MANIFEST_NAME Manifest}
>
> What would you think about linking this to
> {@docRoot}/../specs/jar/jar.html#jar-manifest rather tan
> JarFile#MANIFEST_NAME?
Sure if that is your preference.
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 60:
>
>> 58: * {@link JarEntry#getAttributes()} will return the {@code Manifest}'s
>> 59: * attributes for the current JAR file entry, if any, providing
>> 60: * {@code getManifest()} returns a {@code Manifest} for the JAR file.
>
> Per-entry attributes is an advanced feature to attempt to bring into the
> class description. I think it would be simpler to just drop this paragraph.
> If you really want something on this topic then it would require first
> describing main vs. per entry attributes and then explaining that the
> per-entry attributes are obtained with JarEntry::getAttributes when the
> manifest is at the beginning of the stream.
I can remove, but I am not sure I agree we need to describe main vs attribute
here given we are pointing to the Jar spec and if there is any discussion of
Pre-entry attributes, it should be in JarEntry IMHO. I guess the clarification
I was trying to make, apparently unsuccessfully is that `JarEntry` will not
have access to the attributes if `getManifest` does not return the Manifest.
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 157:
>
>> 155: *
>> 156: * @return the {@code Manifest} for this JAR file when accessible,
>> or
>> 157: * {@code null} otherwise.
>
> The word "accessible" suggests there is access control in the picture so I
> think drop that word. Maybe just drop "if none" from the original return
> description?
Will change as you suggest
-------------
PR: https://git.openjdk.org/jdk/pull/10045