On Tue, 23 Dec 2025 18:00:06 GMT, Chris Plummer <[email protected]> wrote:
>> 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.
Yes, so removing the obscure hidden flag from the comment, it can just be:
void DCmd::register_dcmds() {
// Registration of the diagnostic commands.
// Argument specifies on which interfaces a command is made available.
But as it is called register_dcmds, commenting that we do "Registration of the
diagnostic commands." on the next line is obvious/unnecessary.
Updated!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28962#discussion_r2644095159
