RFR: 8157000: Do not generate javadoc for overridden method with no spec change

[Jonathan Gibbons]

HtmlConfiguration.java
Other public fields nearby have javadoc comments. It would be nice to 
follow the precedent.
That being said, see comments for Utils.java:1568. This option would be 
better defined in
BaseConfiguration, not HtmlConfiguration.

MethodWriterImpl.java 293
No need to assign null. Better to leave it empty (just "Content label;") 
so that the compiler
will verify it is asigned correctly in the statements that follow.

MethodWriterImpl.java 295,296
I acknowledge you've followed the style of 300,301, but as a comment for 
general cleanup
we should move away from legacy wrapper methods like 
configuration.getText in favor
if the more explicit underlying call resources.getText. Or we could do 
that as part of a more
comprehensive cleanup.

resources/standard.properties:371
<...> is normally used for "fill in your own value here", as in <name>, 
<url>, etc
Use (...|...) to indicate alternatives. See line 389 for an example.
         (all|none|[-]<group>)

MemberSumaryBuilder.java:485
Would be good to find a semantically helpful name instead of just 
inhmembers0.

MemberSumaryBuilder.java:486,487,490,517
The short names are sort-of OK, but "members" should be capitalized. 
Also "class" in "inhclass"

MemberSumaryBuilder.java:497, 517
"footnote" is one word, not two. Also implies   addSummaryFootnote

Utils.java:1566
Since this is for methods only, it would be better if the parameter type 
was
ExecutableElement, not Element. That should be OK with the callsite at 
MemberSummaryBuilder:498
Also, the name is not ideal. How about isSimpleOverride?

Utils.java:1568
The need to cast to HtmlConfiguration is a big indication that something 
is defined
in the wrong place. summarizeOveriddenMethods is not HTML-specific and 
so should
be moved to BaseConfiguration.

VisibleMemberMap.java:241,316
Is the cast safe? You're iterating the members, but assuming the member 
is an
ExecutableElement

Test files:
I assume I'm seeing false/duplicate output from the recent repo 
consolidation.
The first group of 3 test files should not be here.

TestOverrideMethods.java
a) if you're going to have long strings in checkOutput, I suggest blank 
lines to help separate the
individual strings.
b) Long strings will be annoying to maintain going forward. Ideally, you 
should reduce the strings
to the minimum necessary to focus on what you need to test, so that 
unrelated work on nearby
content doesn't break these tests.

-- Jon


On 09/21/2017 02:10 PM, Kumar Srinivasan wrote:
> Hello,
>
> Please review for 8157000, please let me know if you have any
> comments or ways I can improve it.
>
> The "before and after" test without the flag is identical.
>
> http://cr.openjdk.java.net/~ksrini/8157000/webrev.00/index.html
>
> Thanks
> Kumar
> Bug: https://bugs.openjdk.java.net/browse/JDK-8157000
> CSR: https://bugs.openjdk.java.net/browse/JDK-8187386