On Mon, 12 Dec 2022 13:06:09 GMT, Alexey Ivanov <[email protected]> wrote:
>> Looks good to me.
>>
>> In the description above: “…invoked one per property…”, it shoud be “once”.
>>
>> I think there's room for improvement. However, this is an internal-only
>> class.
>
>> I think there's room for improvement. However, this is an internal-only
>> class.
>
> This is what I have in mind. The list of callbacks is inconsistent in how the
> method and conditions when it's called are listed. One of the list items has
> no ending punctuation. Some portions of text should use `<code>` or
> `{@code}`, or even
> [`<samp>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/samp).
>
> I think `<p>` should be added before this sentence, _“If an error results in
> parsing, a RuntimeException will be thrown.”_ The sentence itself requires
> re-writing: “If parsing results in an error…”
>
> Both `RuntimeException` and `toLowerCase` below should be in `{@code}`.
>
> The usage of `<code>` could be updated to `{@code}` which is the recommended
> way.
>
> What do you think?
> @aivanov-jdk Done.
Sorry, @scientificware, I missed the update.
After making changes, you have to issue `integrate` command again.
In fact, I didn't mean to include [the changes I
suggested](https://github.com/openjdk/jdk/pull/10975#issuecomment-1346460131)
in this PR.
I still think there's room for improvement, however, it's not visible publicly.
The list is inconsistent:
_“The delegate is notified of the following events:”_ — one would expect a list
of events. Instead, each list item starts differently. If I may suggest,
starting each item with a callback method followed by the condition when it's
called and additional details will present a more consistent documentation. The
introduction sentence before the list may also be updated.
-------------
PR: https://git.openjdk.org/jdk/pull/10975
