On Tue, 4 Feb 2025 10:10:41 GMT, Johan Sjölen <jsjo...@openjdk.org> wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> feedback - test
>
> src/hotspot/share/services/attachListener.cpp line 53:
>
>> 51:
>> 52: // Stream for printing attach operation results.
>> 53: // Supports buffered and streaming output for commands which can produce
>> lenghtly reply.
>
> "lengthy"
Fixed
> src/hotspot/share/services/attachListener.cpp line 56:
>
>> 54: //
>> 55: // To support streaming output platform implementation need to implement
>> AttachOperation::get_reply_writer() method
>> 56: // and ctor allow_streaming argument should be set to true.
>
> Strange mix of "need" and "should".
>
> Can we split this into multiple sentences? Here's a suggestion, though I'm
> not sure if it's 100% correct :-).
>
>
> An output platform implementation supports streaming if it implements
> AttachOperation::get_reply_writer().
> Streaming is enabled if the allow_streaming in the constructor is set to true.
Updated the comment
> src/hotspot/share/services/diagnosticFramework.cpp line 389:
>
>> 387: int count = 0;
>> 388: while (iter.has_next()) {
>> 389: if(_source == DCmd_Source_MBean && count > 0) {
>
> Pre-existing style: Space between `if` and condition missing.
Fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879751
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879963
PR Review Comment: https://git.openjdk.org/jdk/pull/23405#discussion_r1945879828
