On Tue, 23 Dec 2025 15:56:05 GMT, Chris Plummer <[email protected]> wrote:

>> Comment-only cleanup, didn't spot during previous review.
>> 
>> Trivial, no code affected.
>
> src/hotspot/share/services/diagnosticCommand.cpp line 97:
> 
>> 95:   // Registration of the diagnostic commands.
>> 96:   // Argument specifies to which interfaces a command is made available,
>> 97:   // can also specify if hidden from jcmd help.
> 
> I kind a liked the "first argument, second argument" approach a bit more.
> 
> // First argument specifies on which interfaces a command is made available.
> // Optional second argument specifies if hidden from jcmd help. Default is 
> false.

OK thanks.
I was thinking as all the usages here only use one argument, the numbering 
seems irrelevant.
The flags/mask of sources is maybe unusual enough that it's worth keeping the 
comment.

I almost didn't even mention the hidden flag, you go and find the constructor 
DCmdFactoryImpl(uint32_t flags, bool hidden = false) when clarification is 
needed.
Any thoughts on if it's worth mentioning?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28962#discussion_r2643824988

Reply via email to