OK.

The following are style suggestions. Since this review has been ongoing for a while, you don't need to do them now, unless you want to, but be aware of them in future.

On a bunch of methods, like getMemberTreeHeader, you've added javadoc comments, which is good, but the standard is that the first sentence should end with a period.  OK, maybe not so important since this is all internal API, but it would be good to get in the habit of writing correct comments.

You can simplify the style of the code you are writing, by using method chaining on methods on HtmlTree, and/or using varargs overloads.

For example, look at Navigation.java

938 HtmlTree searchDiv = HtmlTree.DIV(HtmlStyle.navListSearch, HtmlTree.LABEL(searchValueId, searchLabel));
939 searchDiv.add(inputText);
940 searchDiv.add(inputReset);

The word "searchDiv exists on all 3 lines.  You can simnplify this to

938 HtmlTree searchDiv = HtmlTree.DIV(HtmlStyle.navListSearch, HtmlTree.LABEL(searchValueId, searchLabel))
939 .add(inputText)
940 .add(inputReset);

You could reasonably add a varargs overload to HtmlTree.DIV, since it is common to create a DIV from a series of Content
elements, and so you could simplify all this, and the next line too, to

938 tree.add(HtmlTree.DIV(HtmlStyle.navListSearch, HtmlTree.LABEL(searchValueId, searchLabel), inputText, inputReset));

That's just one example.

Here's another, in AnnotationTypeFieldWriterImpl.java,

176 Content annotationDetails = new ContentBuilder();
177 annotationDetails.add(annotationDetailsTreeHeader);
178 annotationDetails.add(annotationDetailsTree);

could be just

176 Content annotationDetails = new ContentBuilder().add(annotationDetailsTreeHeader).add(annotationDetailsTree);


or even

176 Content annotationDetails = new ContentBuilder(annotationDetailsTreeHeader, annotationDetailsTree);

In other words, there's a lot of shorter, less verbose forms available these days, for you to use.

-- Jon

On 04/22/2019 01:03 AM, Priya Lakshmi Muthuswamy wrote:
Hi Jon,

I have posted the updated webrev.
Have organized the import statements on the some of the classes(whose imports which were modified).

http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/webrev/
http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/api/
http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/structure/

Thanks,
Priya

On 4/18/2019 4:09 AM, Jonathan Gibbons wrote:
FYI, the patch can not longer be cleanly applied to a repo.

Can you post an updated webrev?

-- Jon


On 04/04/2019 09:24 PM, Priya Lakshmi Muthuswamy wrote:
Hi,

Kindly the review the changes for https://bugs.openjdk.java.net/browse/JDK-8219998

I have made changes in javadoc structure to remove most of the singleton lists in html pages.
Introduced section with the style classes.
Made corresponding changes in stylesheet.

http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/api/
http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/structure/
http://cr.openjdk.java.net/~pmuthuswamy/8219998/review.02/webrev/

Thanks,
Priya




Reply via email to