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
