RFR [14] 8236077: Clean up the use of modifiers and semicolons
Jonathan Gibbons
jonathan.gibbons at oracle.com
Tue Dec 17 19:40:33 UTC 2019
Pavel,
Generally great; thanks for taking this on. It is great to see us
repairing some broken windows [1].
I made some notes while reading the review. None of these require a
followup webrev, unless you
are going to make substantially different /additional updates.
* Style guideline: Use {@code...} in comments instead of <code>...</code>
* Style guideline: Phrases used for the description of @param or
@return should not end with a period
My suggestion for a regex search would be to look for @param @return
tags where the
first character of the description is lower-case and the line ends
with a period.
* Remove a comment containing just {@inheritDoc} when the method is
annotated with @Override
* In general, don't edit localized property files; the L10N team will
do that
* ElementsTable, Utils ... a couple of instances of visitUnknown use
the `p` parameter instead of the
`e` parameter in the AssertionError message
* Style guideline: There's a questionable <h2> with local style
attribute in the jdk.javadoc module info.
The heading could be deleted. The same heading is in the
jdk.compiler module (but that would be
a different fix.) By contrast and way of precedent, the jdk.jartool
module does not have the heading.
If nothing else the local style attribute is probably an anti-pattern.
* doclets/package-info I was going to query your use of "back end"
but I see that dictionaries
and wikipedia accept it. https://en.wikipedia.org/wiki/Back_end, so OK.
* Style guideline: I have come to accept/use Intellij default
formatting of comments, with
the @param tags lined up, and more newlines than the prevailing
practice in the code.
* Style guideline: true, false and null in comments should be in {@code }.
* Style guideline: type names in comments should be in {@code}, but
often, where the type name
is sufficiently descriptive, you can often write the type name as
lower-case English words,
e.g. element, type mirror
-- Jon
[1] https://en.wikipedia.org/wiki/Broken_windows_theory
On 12/17/2019 04:09 AM, Pavel Rappo wrote:
> Hello,
>
> Please review the following trivial change for https://bugs.openjdk.java.net/browse/JDK-8236077:
>
> http://cr.openjdk.java.net/~prappo/8236077/webrev.00/
>
> This is a cleanup task. The above change removes the redundant modifiers (e.g. "public abstract" for
> interface methods). This allows to recover some precious method-signature-line space, reduce visual
> noise, and fix inconsistencies (e.g. where only some of those implicit modifiers were used).
> The remaining modifiers are sorted according to the convention [1]. The change does not alter the
> indentation or param grouping style in the affected methods, unless impractical.
>
> The change also fixes typos in the javadoc comments, comments, variables' and properties' names.
> Not only does this address aesthetic issues, it also helps with searches.
>
> I suggest reviewing this change using a diff tool with a character-level resolution.
> That is, a tool capable of highlighting mismatching characters in lines that differ.
>
> All test/langtools/jdk/javadoc/doclet tests pass. Copyright years will be fixed before the push.
>
> Thanks,
> -Pavel
>
> ---------------------------------------------------------------------------------------------------
> [1] A related task https://bugs.openjdk.java.net/browse/JDK-8136583
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20191217/1af1b0da/attachment-0001.htm>
More information about the javadoc-dev
mailing list