Re: Request Review: JDK-8181639 Add tool and services information to module summary

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Alan Bateman
(Moving this review thread to jigsaw-dev as this is module descriptions).

On 07/06/2017 06:49, Mandy Chung wrote:
> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/webrev.00
>
> Updated javadoc:
>     http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/api/
>
> This patch mainly adds @uses, @provides and description of providers and tools.  This uses description list “dl” to list “Providers”, “Specifications”, “Tool Guides” in JDK 9. In the future this can be converted to custom taglets.
>
I agree a taglet would be useful here, especially as the style specified
to the "dl" tag is copied to many places.

One question for Jon on javadoc: Should the Uses and Provides in the
javadoc print the fully qualified name rather than the simple name? I
found myself several times needing to click on the simple name to see
which area the provider interface is defined in.

I went through all of the updates and have a number of comments/suggestions:

jdk.hotspot.agent
- I think this would be a bit easier to read if we replace "that can
attach" with "to attach".

java.base
- To date, we've used "jrt file system provider" rather than "JRT
FileSystem provider" and it should be good to keep that consistent.
- The URI should be "jrt:/" rather than "jrt".
- What would you think about "to enumerate" rather than "than can
enumerate".
- "can be loaded" -> "can be created".

java.desktop
- I'm in two minds so to whether the @provides should be documented in
this module. This is a case where the docs should make it clear that the
java.desktop includes built-in implementations and list the image and
sound formats that are supported.

java.rmi
- What would you think about trimming this down to:
"The JDK implementation of this module includes the rmiregistry tool to
start a remote object registry, and the rmid tool to start the
activation system daemon". (I've switched the order too because rmid is
less interesting to know about).

java.scripting
- There's a spurious "*" in the description
- Typo "language" -> "languages".

jdk.jartool
- The sentence starting "Instances are ..." follows from the previous
sentence so I think the paragraph break should be removed.
- The statement that jarsigner does not provide an API is confusing as
the module exports two APIs for JAR signing. I think you mean the
ToolProvider and maybe it would be simpler to just drop the sentence.

jdk.jlink
- it might be simpler to list the tools at the start so that it reads:
"Defines the jlink tool for creating run-time images, the jmod tool for
creating and manipulating JMOD files, and the jimage tool for inspecting
the JDK-internal container file for classes and resources". This
ordering then matches the links to the man pages.

jdk.management.agent
- it might be better to say "allows" rather than "enables" as it needs
configuration to enable.

jdk.zipfs
- "and the ability" - I assume this should be "and provides the ability".
- I think we should remove the reference to
FileSystemProviders.installedProviders() as that is not the API that
applications will use to create a zip file system. Instead we can link
to FileSystems.newFileSystem. I also think we need a table in this
javadoc to document the "create" and "encoding" properties.

jdk.security.auth
- your update looks fine but we should probably submit a bug to get
package descriptions added or maybe more text as it's not immediately
obvious what one can do with this module.

jdk.scripting.nashorn
- the existing module-info.java is inconsistent with the other module
descriptions. Now might be the opportunity to reformat this.

jdk.scripting.nashorn.shell
- it might be better to say "the command line tool" rather than "a
command line tool".

I think that is all that I have.

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Mandy Chung
I updated the javadoc per your comment/suggestion.  My reply to some
of your comments are inlined below.

Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/webrev.01/

Module summary pages:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/api/


> On Jun 7, 2017, at 3:36 AM, Alan Bateman <[hidden email]> wrote:
> java.desktop
> - I'm in two minds so to whether the @provides should be documented in this module. This is a case where the docs should make it clear that the java.desktop includes built-in implementations and list the image and sound formats that are supported.
>

That’s a fair point.  I took out @provides and will file a JBS issue
for the client team to document the builtin providers as appropriate.

> jdk.zipfs
> I also think we need a table in this javadoc to document the "create" and "encoding" properties.
>

I agree.  I will file a JBS issue for this.

> jdk.security.auth
> - your update looks fine but we should probably submit a bug to get package descriptions added or maybe more text as it's not immediately obvious what one can do with this module.
>

and jdk.xml.dom as well.  Will file issues to track this.

> jdk.scripting.nashorn
> - the existing module-info.java is inconsistent with the other module descriptions. Now might be the opportunity to reformat this.

I was tempted to reformat it and now I have done it in webrev.01.

Mandy


Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Jonathan Gibbons
In reply to this post by Alan Bateman


On 06/07/2017 03:36 AM, Alan Bateman wrote:
> I agree a taglet would be useful here, especially as the style
> specified to the "dl" tag is copied to many places.

Agreed, but it seemed too late to add that. We'll need to do a round of
cleanup for javadoc support for modules.

-- Jon
Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Lance Andersen
In reply to this post by Mandy Chung
Looks good Mandy

> On Jun 7, 2017, at 1:58 PM, Mandy Chung <[hidden email]> wrote:
>
> I updated the javadoc per your comment/suggestion.  My reply to some
> of your comments are inlined below.
>
> Webrev:
>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/webrev.01/
>
> Module summary pages:
>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/api/
>
>
>> On Jun 7, 2017, at 3:36 AM, Alan Bateman <[hidden email]> wrote:
>> java.desktop
>> - I'm in two minds so to whether the @provides should be documented in this module. This is a case where the docs should make it clear that the java.desktop includes built-in implementations and list the image and sound formats that are supported.
>>
>
> That’s a fair point.  I took out @provides and will file a JBS issue
> for the client team to document the builtin providers as appropriate.
>
>> jdk.zipfs
>> I also think we need a table in this javadoc to document the "create" and "encoding" properties.
>>
>
> I agree.  I will file a JBS issue for this.
>
>> jdk.security.auth
>> - your update looks fine but we should probably submit a bug to get package descriptions added or maybe more text as it's not immediately obvious what one can do with this module.
>>
>
> and jdk.xml.dom as well.  Will file issues to track this.
>
>> jdk.scripting.nashorn
>> - the existing module-info.java is inconsistent with the other module descriptions. Now might be the opportunity to reformat this.
>
> I was tempted to reformat it and now I have done it in webrev.01.
>
> Mandy
>
>

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
[hidden email] <mailto:[hidden email]>



Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Alan Bateman
In reply to this post by Mandy Chung
On 07/06/2017 18:58, Mandy Chung wrote:
> I updated the javadoc per your comment/suggestion.  My reply to some
> of your comments are inlined below.
>
> Webrev:
>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/webrev.01/
>
> Module summary pages:
>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/api/
>
I went through all the module descriptions and everything looks good.
One small typo in java.base where it says "of jrt file ..." when I think
it should be "of the jrt file ..."

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: Request Review: JDK-8181639 Add tool and services information to module summary

Mandy Chung

> On Jun 7, 2017, at 1:39 PM, Alan Bateman <[hidden email]> wrote:
>
> On 07/06/2017 18:58, Mandy Chung wrote:
>> I updated the javadoc per your comment/suggestion.  My reply to some
>> of your comments are inlined below.
>>
>> Webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/webrev.01/
>>
>> Module summary pages:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8181639/api/
>>
> I went through all the module descriptions and everything looks good. One small typo in java.base where it says "of jrt file ..." when I think it should be "of the jrt file …"

Will fix this before pushing it.

Mandy