Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

2019-09-30 Thread Aleks Efimov 

Hey Patrick,

I don't think that we need to add 8229338 to the bug line since it is 
just test refactoring issue (noreg-self).


Otherwise, the changes look good to me!

With Best Regards,
Aleksei

On 27/09/2019 15:53, Patrick Concannon wrote:

Hi Lance,


Thanks for your feedback. I've added in those changes, and you can 
find them in the new webrev linked below.


webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.01/


Kind regards,

Patrick


On 26/09/2019 13:36, Lance Andersen wrote:

Hi Patrick,

Overall I think this looks ok.

A few minor comments

Please add 8229338 to the @bug line

I might suggest adding either a comment to the DataProvider or the 
test which uses it with an overview of the parameters to make it 
easier and quicker for future maintainers to know the intent.


Lines 86 and 91, you could if you want  use String.format and just 
substitute the changed values.


Your testCopy and testFlil methods you can probably consider using a 
DataProvider so that you can also test other types such as Vector or 
was this intentional to omit them ?


HTH

Lance

On Sep 26, 2019, at 4:38 AM, Patrick Concannon 
mailto:patrick.concan...@oracle.com>> 
wrote:


Hi,


Would it be possible to have my fix for JDK-8229338 reviewed?

This a general refactoring of 
test/jdk/java/util/RandomAccess/Basic.java as outlined in 
JDK-8229338 'clean up test/jdk/java/util/RandomAccess/Basic.java'.



Further information on this bug can be found here: 
https://bugs.openjdk.java.net/browse/JDK-8229338


Webrev: 
http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/



Kind regards,

Patrick




 

Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

2019-09-27 Thread Lance Andersen 
Hi Patrick,

This looks much better.

Best
lance
> On Sep 27, 2019, at 10:53 AM, Patrick Concannon 
>  wrote:
> 
> Hi Lance,
> 
> 
> 
> Thanks for your feedback. I've added in those changes, and you can find them 
> in the new webrev linked below.
> 
> webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.01/ 
> 
> 
> Kind regards,
> 
> Patrick
> 
> 
> On 26/09/2019 13:36, Lance Andersen wrote:
>> Hi Patrick,
>> 
>> Overall I think this looks ok.
>> 
>> A few minor comments
>> 
>> Please add 8229338 to the @bug line
>> 
>> I might suggest adding either a comment to the DataProvider or the test 
>> which uses it with an overview of the parameters to make it easier and 
>> quicker for future maintainers to know the intent.
>> 
>> Lines 86 and 91, you could if you want  use String.format and just 
>> substitute the changed values.
>> 
>> Your testCopy and testFlil methods you can probably consider using a 
>> DataProvider so that you can also test other types such as Vector or was 
>> this intentional to omit them ?
>> 
>> HTH
>> 
>> Lance
>> 
>>> On Sep 26, 2019, at 4:38 AM, Patrick Concannon 
>>> mailto:patrick.concan...@oracle.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> 
>>> Would it be possible to have my fix for JDK-8229338 reviewed?
>>> 
>>> This a general refactoring of test/jdk/java/util/RandomAccess/Basic.java as 
>>> outlined in JDK-8229338 'clean up 
>>> test/jdk/java/util/RandomAccess/Basic.java'.
>>> 
>>> 
>>> Further information on this bug can be found here: 
>>> https://bugs.openjdk.java.net/browse/JDK-8229338 
>>> 
>>> 
>>> Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/ 
>>> 
>>> 
>>> 
>>> Kind regards,
>>> 
>>> Patrick
>>> 
>> 
>>  
>> 
>>   
>> 
>>  Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com 
>> 
>> 
>> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

2019-09-27 Thread Patrick Concannon 

Hi Lance,


Thanks for your feedback. I've added in those changes, and you can find 
them in the new webrev linked below.


webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.01/


Kind regards,

Patrick


On 26/09/2019 13:36, Lance Andersen wrote:

Hi Patrick,

Overall I think this looks ok.

A few minor comments

Please add 8229338 to the @bug line

I might suggest adding either a comment to the DataProvider or the 
test which uses it with an overview of the parameters to make it 
easier and quicker for future maintainers to know the intent.


Lines 86 and 91, you could if you want  use String.format and just 
substitute the changed values.


Your testCopy and testFlil methods you can probably consider using a 
DataProvider so that you can also test other types such as Vector or 
was this intentional to omit them ?


HTH

Lance

On Sep 26, 2019, at 4:38 AM, Patrick Concannon 
mailto:patrick.concan...@oracle.com>> 
wrote:


Hi,


Would it be possible to have my fix for JDK-8229338 reviewed?

This a general refactoring of 
test/jdk/java/util/RandomAccess/Basic.java as outlined in JDK-8229338 
'clean up test/jdk/java/util/RandomAccess/Basic.java'.



Further information on this bug can be found here: 
https://bugs.openjdk.java.net/browse/JDK-8229338


Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/


Kind regards,

Patrick





Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: RFR[8229338]: clean up test/jdk/java/util/RandomAccess/Basic.java

2019-09-26 Thread Lance Andersen 
Hi Patrick,

Overall I think this looks ok.

A few minor comments

Please add 8229338 to the @bug line

I might suggest adding either a comment to the DataProvider or the test which 
uses it with an overview of the parameters to make it easier and quicker for 
future maintainers to know the intent.

Lines 86 and 91, you could if you want  use String.format and just substitute 
the changed values.

Your testCopy and testFlil methods you can probably consider using a 
DataProvider so that you can also test other types such as Vector or was this 
intentional to omit them ?

HTH

Lance

> On Sep 26, 2019, at 4:38 AM, Patrick Concannon  
> wrote:
> 
> Hi,
> 
> 
> Would it be possible to have my fix for JDK-8229338 reviewed?
> 
> This a general refactoring of test/jdk/java/util/RandomAccess/Basic.java as 
> outlined in JDK-8229338 'clean up test/jdk/java/util/RandomAccess/Basic.java'.
> 
> 
> Further information on this bug can be found here: 
> https://bugs.openjdk.java.net/browse/JDK-8229338
> 
> Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/
> 
> 
> Kind regards,
> 
> Patrick
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com