On Fri, 17 Sep 2021 20:56:14 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Renamed JDITask to NamedTask.
>
> test/hotspot/jtreg/vmTestbase/nsk/jdi/BreakpointRequest/addInstanceFilter/instancefilter004a.java
>  line 154:
> 
>> 152:     static class Threadinstancefilter004a extends JDITask {
>> 153: 
>> 154:         String tName = null;
> 
> Why did you keep tName? In previous similar tests you got rid of it and 
> relied on getName().

Too late realized that it is redundant and forget to fix some tests.
Updated and checked that tName is used anymore.

> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/JDITask.java line 26:
> 
>> 24: package nsk.share.jdi;
>> 25: 
>> 26: public abstract class JDITask implements Runnable {
> 
> I was thinking maybe `NamedTask` or `NamedRunnable` might be a better and 
> more generic name, unless you think you'll be enhancing this class, but 
> `JDITask` is ok if you want to keep it.

Renamed to NamedTask.

> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/JDIThreadFactory.java line 39:
> 
>> 37:         Thread t = threadFactory.newThread(task);
>> 38:         t.setName(task.getName());
>> 39:         return t;
> 
> How about:
> `return newThread(task, task.getName());`

fixed

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

PR: https://git.openjdk.java.net/jdk/pull/5515

Reply via email to