On Tue, 23 Dec 2025 17:14:33 GMT, Kevin Walls <[email protected]> wrote:

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

Yeah, before adding my comment I had to first go digging for that `bool hidden 
= false` reference. At first it was unclear to me what the comment was even 
trying to describe. I think if you wanted to delete the comment that would be 
ok. I don't think we have a comment like this is any of the other numerous 
sections that register dcmds.

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

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

Reply via email to