Quantcast

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Mandy Chung
I edited the module descriptions per your feedback.  I also revised
GenGraphs tool to take a properties file to customize the dot graphs
for javadoc use.

Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/

Magnus, Erik,

I modified Javadoc.gmk and Main.gmk to add a new target to generate
.dot files for javadoc use.  GenGraphs tool depends on the exploded
image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
variable for us to enable @moduleGraph taglet until JDK-8176785 is
resolved.

thanks
Mandy

> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> wrote:
>
> On 24/03/2017 21:50, Mandy Chung wrote:
>
>> Alan,
>>
>> I took another round of edits on the module descriptions:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>
>> Once we have the unified docs, it will make it easier to review
>> the module summary page for all modules where we will revise
>> these module descriptions again.
>>
> I went through the updated module descriptions.
>
> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>

Fixed. I missed that.

> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>

This reads better.

> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>

Let’s start with this version.  I expect we will refine the module
description further next couple weeks.

> Everything else looks good.


Thanks
Mandy
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Erik Joelsson
Looks ok to me. Not sure about adding a make variable as a parameter,
but I assume Magnus will fix that as part of JDK-8176785?

/Erik


On 2017-03-25 22:33, Mandy Chung wrote:

> I edited the module descriptions per your feedback.  I also revised
> GenGraphs tool to take a properties file to customize the dot graphs
> for javadoc use.
>
> Updated webrev:
>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
>
> Magnus, Erik,
>
> I modified Javadoc.gmk and Main.gmk to add a new target to generate
> .dot files for javadoc use.  GenGraphs tool depends on the exploded
> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
> variable for us to enable @moduleGraph taglet until JDK-8176785 is
> resolved.
>
> thanks
> Mandy
>
>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> wrote:
>>
>> On 24/03/2017 21:50, Mandy Chung wrote:
>>
>>> Alan,
>>>
>>> I took another round of edits on the module descriptions:
>>>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>
>>> Once we have the unified docs, it will make it easier to review
>>> the module summary page for all modules where we will revise
>>> these module descriptions again.
>>>
>> I went through the updated module descriptions.
>>
>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>>
> Fixed. I missed that.
>
>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>>
> This reads better.
>
>> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>>
> Let’s start with this version.  I expect we will refine the module
> description further next couple weeks.
>
>> Everything else looks good.
>
> Thanks
> Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Mandy Chung

> On Mar 27, 2017, at 1:38 AM, Erik Joelsson <[hidden email]> wrote:
>
> Looks ok to me. Not sure about adding a make variable as a parameter, but I assume Magnus will fix that as part of JDK-8176785?
>

Thanks.  The make variable is temporary and should be removed with JDK-817678.

Mandy

> /Erik
>
>
> On 2017-03-25 22:33, Mandy Chung wrote:
>> I edited the module descriptions per your feedback.  I also revised
>> GenGraphs tool to take a properties file to customize the dot graphs
>> for javadoc use.
>>
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
>>
>> Magnus, Erik,
>>
>> I modified Javadoc.gmk and Main.gmk to add a new target to generate
>> .dot files for javadoc use.  GenGraphs tool depends on the exploded
>> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
>> variable for us to enable @moduleGraph taglet until JDK-8176785 is
>> resolved.
>>
>> thanks
>> Mandy
>>
>>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> wrote:
>>>
>>> On 24/03/2017 21:50, Mandy Chung wrote:
>>>
>>>> Alan,
>>>>
>>>> I took another round of edits on the module descriptions:
>>>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>>
>>>> Once we have the unified docs, it will make it easier to review
>>>> the module summary page for all modules where we will revise
>>>> these module descriptions again.
>>>>
>>> I went through the updated module descriptions.
>>>
>>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>>>
>> Fixed. I missed that.
>>
>>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>>>
>> This reads better.
>>
>>> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>>>
>> Let’s start with this version.  I expect we will refine the module
>> description further next couple weeks.
>>
>>> Everything else looks good.
>>
>> Thanks
>> Mandy
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Magnus Ihse Bursie
In reply to this post by Mandy Chung
On 2017-03-25 22:33, Mandy Chung wrote:

> I edited the module descriptions per your feedback.  I also revised
> GenGraphs tool to take a properties file to customize the dot graphs
> for javadoc use.
>
> Updated webrev:
>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
>
> Magnus, Erik,
>
> I modified Javadoc.gmk and Main.gmk to add a new target to generate
> .dot files for javadoc use.  GenGraphs tool depends on the exploded
> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
> variable for us to enable @moduleGraph taglet until JDK-8176785 is
> resolved.

Looks good to me.

/Magnus

>
> thanks
> Mandy
>
>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> wrote:
>>
>> On 24/03/2017 21:50, Mandy Chung wrote:
>>
>>> Alan,
>>>
>>> I took another round of edits on the module descriptions:
>>>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>
>>> Once we have the unified docs, it will make it easier to review
>>> the module summary page for all modules where we will revise
>>> these module descriptions again.
>>>
>> I went through the updated module descriptions.
>>
>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>>
> Fixed. I missed that.
>
>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>>
> This reads better.
>
>> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>>
> Let’s start with this version.  I expect we will refine the module
> description further next couple weeks.
>
>> Everything else looks good.
>
> Thanks
> Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Magnus Ihse Bursie
In reply to this post by Mandy Chung
On 2017-03-25 22:33, Mandy Chung wrote:
> I edited the module descriptions per your feedback.  I also revised
> GenGraphs tool to take a properties file to customize the dot graphs
> for javadoc use.
>
> Updated webrev:
>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
I just discovered a problem with this patch when I tried to apply it
locally.

With this patch, it is not possible to run "make docs-javadoc" in a
clean build. The problem is that ModuleTools.gmk is broken, and
overrides the value of BUILD_TOOLS_JDK from Tools.gmk.

/Magnus

> Magnus, Erik,
>
> I modified Javadoc.gmk and Main.gmk to add a new target to generate
> .dot files for javadoc use.  GenGraphs tool depends on the exploded
> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
> variable for us to enable @moduleGraph taglet until JDK-8176785 is
> resolved.
>
> thanks
> Mandy
>
>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> wrote:
>>
>> On 24/03/2017 21:50, Mandy Chung wrote:
>>
>>> Alan,
>>>
>>> I took another round of edits on the module descriptions:
>>>    http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>
>>> Once we have the unified docs, it will make it easier to review
>>> the module summary page for all modules where we will revise
>>> these module descriptions again.
>>>
>> I went through the updated module descriptions.
>>
>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>>
> Fixed. I missed that.
>
>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>>
> This reads better.
>
>> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>>
> Let’s start with this version.  I expect we will refine the module
> description further next couple weeks.
>
>> Everything else looks good.
>
> Thanks
> Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Magnus Ihse Bursie
On 2017-03-29 13:45, Magnus Ihse Bursie wrote:

> On 2017-03-25 22:33, Mandy Chung wrote:
>> I edited the module descriptions per your feedback.  I also revised
>> GenGraphs tool to take a properties file to customize the dot graphs
>> for javadoc use.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/
> I just discovered a problem with this patch when I tried to apply it
> locally.
>
> With this patch, it is not possible to run "make docs-javadoc" in a
> clean build. The problem is that ModuleTools.gmk is broken, and
> overrides the value of BUILD_TOOLS_JDK from Tools.gmk.

Suggested patch, which solves the problem:

diff --git a/make/ModuleTools.gmk b/make/ModuleTools.gmk
--- a/make/ModuleTools.gmk
+++ b/make/ModuleTools.gmk
@@ -1,5 +1,5 @@
  #
-# Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights
reserved.
+# Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights
reserved.
  # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  #
  # This code is free software; you can redistribute it and/or modify it
@@ -23,8 +23,9 @@
  # questions.
  #

-include $(SPEC)
-include MakeBase.gmk
+ifndef _MODULE_TOOLS_GMK
+_MODULE_TOOLS_GMK := 1
+
  include JavaCompilation.gmk

  TOOLS_CLASSES_DIR := $(BUILDTOOLS_OUTPUTDIR)/tools_jigsaw_classes
@@ -32,7 +33,7 @@
  # To avoid reevaluating the compilation setup for the tools each time
this file
  # is included, the actual compilation is handled by
CompileModuleTools.gmk. The
  # following trick is used to be able to declare a dependency on the
built tools.
-BUILD_TOOLS_JDK := $(call SetupJavaCompilationCompileTarget, \
+BUILD_JIGSAW_TOOLS := $(call SetupJavaCompilationCompileTarget, \
      BUILD_JIGSAW_TOOLS, $(TOOLS_CLASSES_DIR))

  TOOL_GENGRAPHS := $(BUILD_JAVA) -esa -ea -cp $(TOOLS_CLASSES_DIR) \
@@ -47,3 +48,5 @@
      -cp $(TOOLS_CLASSES_DIR) \
      --add-exports java.base/jdk.internal.module=ALL-UNNAMED \
      build.tools.jigsaw.AddPackagesAttribute
+
+endif # _MODULE_TOOLS_GMK

/Magnus

>
> /Magnus
>
>> Magnus, Erik,
>>
>> I modified Javadoc.gmk and Main.gmk to add a new target to generate
>> .dot files for javadoc use.  GenGraphs tool depends on the exploded
>> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
>> variable for us to enable @moduleGraph taglet until JDK-8176785 is
>> resolved.
>>
>> thanks
>> Mandy
>>
>>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]>
>>> wrote:
>>>
>>> On 24/03/2017 21:50, Mandy Chung wrote:
>>>
>>>> Alan,
>>>>
>>>> I took another round of edits on the module descriptions:
>>>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/
>>>>
>>>> Once we have the unified docs, it will make it easier to review
>>>> the module summary page for all modules where we will revise
>>>> these module descriptions again.
>>>>
>>> I went through the updated module descriptions.
>>>
>>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I
>>> think they will read okay once that is added.
>>>
>> Fixed. I missed that.
>>
>>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP
>>> server", it might be better to re-shuffle this to "Defines the API
>>> for the JDK-specific HTTP server”.
>>>
>> This reads better.
>>
>>> I think the only one that needs re-examination is jdk.charsets. The
>>> java.base module contains the standard charsets and all other
>>> charsets needed to start the runtime on any of the supported
>>> configurations. It thus varies by platform with jdk.charsets
>>> providing the charsets that aren't in java.base. Finding the right
>>> description is difficult, maybe we should start with "Charset
>>> provider for the charsets that are not in java.base (mostly double
>>> byte and IBM charsets". I could imagine linking this to the
>>> "Supported encodings" docs page in time.
>>>
>> Let’s start with this version.  I expect we will refine the module
>> description further next couple weeks.
>>
>>> Everything else looks good.
>>
>> Thanks
>> Mandy
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Review Request: JDK-8173303: Add module-subgraph images to main platform documentation

Mandy Chung
In reply to this post by Magnus Ihse Bursie
I fixed that but the webrev.02 missed to include this line fix (sorry about that):

-BUILD_TOOLS_JDK := $(call SetupJavaCompilationCompileTarget, \
+BUILD_JIGSAW_TOOLS := $(call SetupJavaCompilationCompileTarget, \
     BUILD_JIGSAW_TOOLS, $(TOOLS_CLASSES_DIR))

I’ll include ifndef as you suggested.
 
Mandy

> On Mar 29, 2017, at 4:45 AM, Magnus Ihse Bursie <[hidden email]> wrote:
>
> On 2017-03-25 22:33, Mandy Chung wrote:
>> I edited the module descriptions per your feedback.  I also revised
>> GenGraphs tool to take a properties file to customize the dot graphs
>> for javadoc use.
>>
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/ <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.02/>I just discovered a problem with this patch when I tried to apply it locally.
>
> With this patch, it is not possible to run "make docs-javadoc" in a clean build. The problem is that ModuleTools.gmk is broken, and overrides the value of BUILD_TOOLS_JDK from Tools.gmk.
>
> /Magnus
>
>> Magnus, Erik,
>>
>> I modified Javadoc.gmk and Main.gmk to add a new target to generate
>> .dot files for javadoc use.  GenGraphs tool depends on the exploded
>> image build.  Javadoc.gmk temporarily takes ENABLE_MODULE_GRAPH make
>> variable for us to enable @moduleGraph taglet until JDK-8176785 is
>> resolved.
>>
>> thanks
>> Mandy
>>
>>> On Mar 25, 2017, at 2:36 AM, Alan Bateman <[hidden email]> <mailto:[hidden email]> wrote:
>>>
>>> On 24/03/2017 21:50, Mandy Chung wrote:
>>>
>>>> Alan,
>>>>
>>>> I took another round of edits on the module descriptions:
>>>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/ <http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173303/webrev.01/>
>>>>
>>>> Once we have the unified docs, it will make it easier to review
>>>> the module summary page for all modules where we will revise
>>>> these module descriptions again.
>>>>
>>> I went through the updated module descriptions.
>>>
>>> Two more that seem to be missing "the" are jdk.net and jdk.sctp, I think they will read okay once that is added.
>>>
>> Fixed. I missed that.
>>
>>> jdk.httpserver currently has "Defines the JDK-specific API for HTTP server", it might be better to re-shuffle this to "Defines the API for the JDK-specific HTTP server”.
>>>
>> This reads better.
>>
>>> I think the only one that needs re-examination is jdk.charsets. The java.base module contains the standard charsets and all other charsets needed to start the runtime on any of the supported configurations. It thus varies by platform with jdk.charsets providing the charsets that aren't in java.base. Finding the right description is difficult, maybe we should start with "Charset provider for the charsets that are not in java.base (mostly double byte and IBM charsets". I could imagine linking this to the "Supported encodings" docs page in time.
>>>
>> Let’s start with this version.  I expect we will refine the module
>> description further next couple weeks.
>>
>>> Everything else looks good.
>>
>> Thanks
>> Mandy
>

Loading...