On Fri, 22 Mar 2024 20:27:31 GMT, Daniel D. Daugherty <dcu...@openjdk.org> 
wrote:

>> Alex Menkov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   feedback
>
> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java
>  line 117:
> 
>> 115:                     boolean isConnected = true;
>> 116:                     boolean eventsReceived = false;
>> 117:                     // handle events until debugee is disconnected
> 
> Nit typo: s/debugee/debuggee/
> 
> But a lot of the NSK tests have this typo...

Fixed.

> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/referenceType/refType001.java
>  line 207:
> 
>> 205:                                                   eventsReceived = true;
>> 206:                                                   for (int i = 0; i < 
>> checkedTypes.length; i++) {
>> 207:                                                       if 
>> (checkedTypes[i][2] == "0") {
> 
> This if-statement could use a comment to explain the logic.

Done.

> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java 
> line 138:
> 
>> 136:                     boolean isConnected = true;
>> 137:                     boolean eventsReceived = false;
>> 138:                     // handle events until debugee is disconnected
> 
> Nit typo: s/debugee/debuggee/
> 
> But a lot of the NSK tests have this typo...

Fixed.

> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java 
> line 233:
> 
>> 231:                                           }
>> 232: 
>> 233:                                           // Check that all expected 
>> ClassPrepareEvent are received
> 
> nit typo: s/ClassPrepareEvent/ClassPrepareEvent(s)/
> 
> nit typo: Since this comment starts with a capital, it should have a period 
> at the end.

Fixed.

> test/hotspot/jtreg/vmTestbase/nsk/jdi/ClassPrepareEvent/thread/thread001.java 
> line 237:
> 
>> 235:                                               eventsReceived = true;
>> 236:                                               for (int i = 0; i < 
>> checkedThreads.length; i++) {
>> 237:                                                   if 
>> (checkedThreads[i][2] == "0") {
> 
> This if-statement could use a comment to explain the logic.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536241165
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536241466
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536241591
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536241959
PR Review Comment: https://git.openjdk.org/jdk/pull/18442#discussion_r1536242129

Reply via email to