RFR [15] 8243563: Doc comments cleanup

Pavel Rappo pavel.rappo at oracle.com
Fri Apr 24 17:42:22 UTC 2020


> On 24 Apr 2020, at 18:03, Erik Gahlin <erik.gahlin at oracle.com> wrote:
> 
> Hi Pavel,
> 
> Awesome work! Thanks for doing this!
> 
> I wish I would have tried to apply {@literal } to a larger segment of code, it would have simplified things.

Before yesterday, when that JavaDoc change (JDK-8241780) was pushed, there had been a possibility of missing an @ symbol if a scope of @code/@literal was too broad. Well, the good news is that you should no longer be bothered by this.

> I noticed you changed from {@code } to {@link}. I assume the rule is that the first mention should use {@link} and the remaining uses {@code}?

Right. Though, it is not a rule but rather a recommendation from an old Style Guide (see https://www.oracle.com/technetwork/java/javase/documentation/index-137868.html):

"
* Use in-line links economically

You are encouraged to add links for API names (listed immediately above) using the {@link} tag. It is not necessary to add links for all API names in a doc comment. Because links call attention to themselves (by their color and underline in HTML, and by their length in source code doc comments), it can make the comments more difficult to read if used profusely. We therefore recommend adding a link to an API name if:

  * The user might actually want to click on it for more information (in your judgment), and
  * Only for the first occurrence of each API name in the doc comment (don't bother repeating a link)

Our audience is advanced (not novice) programmers, so it is generally not necessary to link to API in the java.lang package (such as String), or other API you feel would be well-known.
"

> I noticed that you changed the code sample in RecordingStream (L347) from event -> System.out-println(event) to System.out::println. While I agree the latter form is usually preferred, I think in this case it helps if you can see the callback object is an event.

I changed that because of inconsistency with the remaining examples (5 more of them) in that same file. I reckon that the startAsync method is quite new. This is probably why it is documented a bit differently from the rest of the methods of RecordingStream. That said, I'll revert that bit if you want.

> Otherwise, looks good!
> 
> Thanks
> Erik
> 
>> On 24 Apr 2020, at 17:40, Pavel Rappo <pavel.rappo at oracle.com> wrote:
>> 
>> Dear jfr folk,
>> 
>> Could you consider and review the changeset for https://bugs.openjdk.java.net/browse/JDK-8243563
>> 
>> http://cr.openjdk.java.net/~prappo/8243563/webrev.00/
>> 
>> This is a cleanup that was triggered by the recent change in the way JavaDoc processes inline tags [1]. In a nutshell, JavaDoc has allowed to begin a line with @ symbol inside inline tags such as {@code} and {@literal}.
>> 
>> While testing that change Jon Gibbons noticed that the resulting documentation was slightly different. One of the differences was caused by unterminated inline tag, a typo in the doc comment for the EventStream#setReuse(boolean) method [2]:
>> 
>> * If reuse is set to {@code true), an action should not keep a reference
>> 
>> I fixed that and then decided to try out that JavaDoc change by blanketly retrofitting the jdk.jfr doc comments. Regretfully for reviewers, I've done a bit more than I anticipated. Not only do the webrev above fixes that unterminated tag and retrofits the doc comments, it also fixes typos, grammar, and comment skew (including non-compiling examples).
>> 
>> Fixes of typos and grammar are mostly in comments and some are in error messages and *code constructs* such as classes, enum constants, fields, methods, and local variables. I think I was careful and made sure those were in non-exported packages. However, please keep that in mind and pay extra attention while reviewing the change.
>> 
>> The only thing I couldn't fix was the examples from the doc comment for the Recording.setSettings method. I'd appreciate if the experts have a look at and update it.
>> 
>> -Pavel
>> 
>> -------------------
>> [1] https://bugs.openjdk.java.net/browse/JDK-8241780
>> [2] http://hg.openjdk.java.net/jdk/jdk/file/cf8b50c400be/src/jdk.jfr/share/classes/jdk/jfr/consumer/EventStream.java#l276
>> 
> 



More information about the hotspot-jfr-dev mailing list