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

Lance Andersen lance.andersen at oracle.com
Fri Sep 27 18:31:11 UTC 2019


Hi Patrick,

This looks much better.

Best
lance
> On Sep 27, 2019, at 10:53 AM, Patrick Concannon <patrick.concannon at 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.concannon at oracle.com <mailto:patrick.concannon at 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.Andersen at oracle.com <mailto:Lance.Andersen at 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.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>





More information about the core-libs-dev mailing list