RFR(s): 8165621: Convert TestG1BiasedArray_test to GTest

Dmitry Fazunenko dmitry.fazunenko at oracle.com
Thu Oct 6 06:16:39 UTC 2016


Jesper, thanks for review!

-- Dima


On 06.10.2016 8:33, Jesper Wilhelmsson wrote:
> Den 5/10/16 kl. 18:20, skrev Dmitry Fazunenko:
>> Jesper,
>>
>> Thanks for looking!
>> new webrevs:
>> http://cr.openjdk.java.net/~dfazunen/8165621/webrev.02/
>> http://cr.openjdk.java.net/~dfazunen/8165621/webrev.01vs02/
>>
>>> The copyright date in the new test should be 2016 only.
>> No, I'm pretty sure the copyright should include two years, because 
>> the IP
>> (Intellectual property) was created earlier than the file.
>
> Yes, you are absolutely right! I verified this with RT and the start 
> date should follow from the old file. This means that some other 
> converted tests are missing the start date. I'll dig into that.
>
> New version looks good!
> /Jesper
>
>>
>> Thanks,
>> Dima
>>
>>
>> On 05.10.2016 18:42, Jesper Wilhelmsson wrote:
>>> Looks good in general.
>>>
>>> The copyright date in the new test should be 2016 only.
>>>
>>> Also in the same file there are some line breaks that I don't find 
>>> motivated.
>>> There is no need to keep the lines below 80 characters in general. A 
>>> line can
>>> easily be 100-120 characters without being too long to read.
>>>
>>> Lines 43-44 and similar further down in the file: To me it makes the 
>>> code
>>> harder to read when split in this way. I would prefer a single 
>>> longer line
>>> when the saving by splitting is not more than 15-20 characters.
>>>
>>> Lines 52-56 is an extreme case which will imho be much easier to 
>>> read if
>>> merged into two or three lines.
>>>
>>> Thanks,
>>> /Jesper
>>>
>>>
>>> Den 5/10/16 kl. 16:44, skrev Dmitry Fazunenko:
>>>> Thank you, Kirill.
>>>>
>>>> Does anyone else want to take a look?
>>>>
>>>> -- Dima
>>>>
>>>> On 05.10.2016 17:47, Kirill Zhaldybin wrote:
>>>>> Dmitry,
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> Regards, Kirill
>>>>>
>>>>> On 27.09.2016 19:22, Dmitry Fazunenko wrote:
>>>>>> Hello,
>>>>>>
>>>>>> may I have a couple of reviews for a change related to conversion 
>>>>>> of an
>>>>>> internal VM test to GTest, please.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8165621
>>>>>> http://cr.openjdk.java.net/~dfazunen/8165621/webrev.01/
>>>>>>
>>>>>> Thanks,
>>>>>> Dima
>>>>>
>>>>
>>




More information about the hotspot-gc-dev mailing list