RFR (s): 8147003: Move BubbleUpRef test into CMS directory
Dmitry Fazunenko
dmitry.fazunenko at oracle.com
Wed Feb 3 15:54:45 UTC 2016
Thank you, Bengt!
On 03.02.2016 18:17, Bengt Rutisson wrote:
>
> Hi Dima,
>
> Thanks for you comments. Makes sense. And thanks for providing the diff.
>
> Reviewed.
>
> Bengt
>
>
> On 2016-02-03 15:38, Dmitry Fazunenko wrote:
>> Hi Bengt,
>>
>> Thanks a lot for review and the comments provided.
>>
>> On 03.02.2016 17:02, Bengt Rutisson wrote:
>>>
>>> Hi Dima,
>>>
>>> On 2016-02-03 14:24, Dmitry Fazunenko wrote:
>>>> Hi all,
>>>>
>>>> May I have a couple of reviews of a simple change:
>>>>
>>>> http://cr.openjdk.java.net/~dfazunen/8147003/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8147003
>>>>
>>>> Background:
>>>> This test is not new, I just took the source of existing closed
>>>> test, performed some cosmetic changes and put in open.
>>>> The original logic has preserved unchanged.
>>>
>>> I'm fine with pushing this, but I would have preferred if the test
>>> would just have been moved and the cosmetic changes would have been
>>> done as a separate change. Not all of the cosmetic changes are
>>> obvious improvements. I am for example not sure why you wanted to
>>> remove the help text that was logged if the test was started with
>>> the wrong arguments.
>>
>> I moved the help text from the code into method description. It's
>> very unlikely that code will be ever executed because the arguments
>> are hardcoded.
>> So, it's better to have test argument description in the comments.
>>
>>>
>>> As I said, unless anybody else thinks otherwise I am ok with this
>>> change. But I would prefer if future changes like this was separated
>>> in to a move that is as clean as possible and then a separate change
>>> to clean the code up.
>>
>> I was thinking about making the change in multiple steps, but decided
>> that it would be over complicated for such a simple thing.
>> Okay, I got your point for the future.
>>
>> I attached the diff between old and new version. It's hard to read,
>> because of indent changes, so I didn't attach it earlier.
>>
>> Thanks,
>> Dima
>>
>>
>>>
>>> Thanks,
>>> Bengt
>>>
>>>>
>>>> Thanks,
>>>> Dima
>>>
>>
>
More information about the hotspot-gc-dev
mailing list