Hi Patrick,

This looks much better.

Best
lance
> On Sep 27, 2019, at 10:53 AM, Patrick Concannon 
> <patrick.concan...@oracle.com> 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/ 
> <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 
>>> <patrick.concan...@oracle.com <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 
>>> <https://bugs.openjdk.java.net/browse/JDK-8229338>
>>> 
>>> Webrev: http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/ 
>>> <http://cr.openjdk.java.net/~pconcannon/8229338/webrevs/webrev.00/>
>>> 
>>> 
>>> Kind regards,
>>> 
>>> Patrick
>>> 
>> 
>> <oracle_sig_logo.gif> 
>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif> 
>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>  <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
>> 
>> 
>> 

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> 
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>



Reply via email to