On Mon, 26 Jul 2021 13:55:45 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
> This PR corrects/adds missing documentation for classes in javafx.css package.
I applied the patch and confirm that there are no more "missing comments"
warnings for the `javafx.css` package.
The docs look reasonable in general. I did note a few examples of usage
patterns that should be fixed globally. Most of these fell into the following
general categories.
1. The body of the javadoc comments for a class, method, etc., should be one or
more complete sentences starting with a capital letter and ending with a period.
2. The text following `@return` and `@param` typically starts with a lower-case
letter and does not end with a period.
3. When referring to a class name, method name, or formal parameter name, we
usually use `{@code ... }` style or else separate words that are all
lower-case. For example, either `{@code ParsedValue}` or `parsed value` is fine
(but not `ParsedValue`).
In addition to noting one or two examples of each of the above, I left a few
random inline comments.
A second pair of eyes would be good for these changes.
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4724:
> 4722:
> 4723: /**
> 4724: * List of errors that may have occurred during css processing.
Maybe `Returns a list of errors...`? Also, CSS should probably be all-caps for
consistency.
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4725:
> 4723: /**
> 4724: * List of errors that may have occurred during css processing.
> 4725: * @return An {@code ObservableList} of {@code ParseError}
We typically start the text following `@return` or `@param` with a lower-case
letter. So: `an ...`
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4739:
> 4737: public static class ParseError {
> 4738:
> 4739: /**
Can you also add a one-sentence description, something like: `Returns the error
message.`?
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4740:
> 4738:
> 4739: /**
> 4740: * @return The error message from the CSS code.
We typically start with a lower can letter and omit the period at the end of
`@return` or `@param`. Also, I'm not sure that "from the CSS code" is needed.
So maybe:
* @return the error message
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4748:
> 4746: /**
> 4747: * Constructs a {@code ParseError} object with given message.
> 4748: * @param message message
Maybe "the message"?
modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 4835:
> 4833: /**
> 4834: * Constructs a {@code PropertySetError} object.
> 4835: * @param styleableProperty css meta data
css -> CSS
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 36:
> 34:
> 35: /**
> 36: * This class serves as a container of CSS-property and it's value.
`CSS-property` should not be hyphenated, and it should be _a_ CSS property.
Also: `it's` --> `its`.
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 67:
> 65: /**
> 66: * Get the parsed value
> 67: * @return ParsedValue
Either `@return the {@code ParsedValue}` or `@return the parsed value`
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 74:
> 72:
> 73: /**
> 74: * Get the CSS property name
End with a period. And we usually would say "Gets" rather than "Get" here.
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 75:
> 73: /**
> 74: * Get the CSS property name
> 75: * @return css-property
For consistency, I'd say: `@return the CSS property name`
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 108:
> 106: }
> 107: /**
> 108: * One declaration is equal to another regardless of the {@code
> Rule} to which
This is a good second sentence. The first sentence should probably be copied
from the `Object::equals` method:
Indicates whether some other object is "equal to" this one.
Since this is pre-existing, it would be fine to not do this as part of this bug.
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 140:
> 138: /**
> 139: * Returns the hash code of this {@code Declaration}
> 140: */
Since there is nothing extra to say beyond what the superclass says, you can
instead replace this with:
/** {@inheritDoc} */
If you do want to add an explicit javadoc comment here, you need an `@return`
tag.
modules/javafx.graphics/src/main/java/javafx/css/Declaration.java line 151:
> 149: /**
> 150: * Returns a String version of this {@code Declaration}
> 151: */
Same comment as for `hashCode`. Probably best to use use:
/** {@inheritDoc} */
modules/javafx.graphics/src/main/java/javafx/css/PseudoClass.java line 85:
> 83: /**
> 84: * Gets the {@code PseudoClass} instance for a given pseudo class
> name.
> 85: * Note : There is only one PseudoClass instance for a given pseudo
> class name.
Maybe put this in a new paragraph? Also, can you remove the space before the
`:`?
modules/javafx.graphics/src/main/java/javafx/css/SizeUnits.java line 38:
> 36:
> 37: /**
> 38: * Percentage
Maybe make this a complete sentence by saying something like `Represents a size
as a percentage.` for this one? For the concrete units, something like
`Represents a size in inches.` (for example).
modules/javafx.graphics/src/main/java/javafx/css/SizeUnits.java line 138:
> 136:
> 137: /**
> 138: * EX
What does "EX" specify? Since it isn't an obvious unit type, it would be good
to define it.
modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 28:
> 26:
> 27: /**
> 28: * A class that contains StyleClass information
Best to use `{@code StyleClass}`. Also needs to end with a period.
modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 45:
> 43: /**
> 44: * Returns the name of StyleClass.
> 45: * @return the style-class
`StyleClass` --> `the {@code StyleClass)` or `the style-class`.
modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 60:
> 58:
> 59: /**
> 60: * Returns the index of this StyleClass in styleClasses list.
Where is the `styleClasses` list defined? This may be beyond the scope of this
fix, so a follow-on issue could be filed. At the least, you should add "the"
before "styleClasses".
modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java
line 83:
> 81: /**
> 82: * Converter to convert DropShadow {@code Effect}
> 83: * @since 9
Good catch on the `@since 9` tag. I'm a little surprised it doesn't inherit the
`@since` from the enclosing class if not present in the nested class, but I can
see that it doesn't. So we'll to check the docs of all nested classes to make
sure that there aren't any missing.
-------------
PR: https://git.openjdk.java.net/jfx/pull/589
