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