On Wed, 25 Jan 2023 19:29:32 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>>> 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.

@aivanov-jdk Thanks.

-------------

PR: https://git.openjdk.org/jdk/pull/10975

Reply via email to