RFR (s): 8026849 - Fix typos in the GC code, part 2

Coleen Phillimore coleen.phillimore at oracle.com
Thu Jan 30 07:04:40 PST 2014


The changes without the comma look good.
Coleen

On 1/30/14 9:24 AM, Jesper Wilhelmsson wrote:
> Kirk Pepperdine skrev 30/1/14 2:44 PM:
>> Hi Jesper,
>>
>> Understood. Did this people express their views in a forum where we 
>> all can see and understand why this comma is so important to them?
>
> No, it was in private communication. I'll ask them to comment on this 
> issue in the new review thread.
> /Jesper
>
>>
>> Regards,
>> Kirk
>>
>> On Jan 30, 2014, at 1:20 PM, Jesper Wilhelmsson 
>> <jesper.wilhelmsson at oracle.com> wrote:
>>
>>> Hi again,
>>>
>>> I got comments from several people that the comma-fix described in 
>>> 8023899 really should be fixed, so I'm reopening the bug. I will not 
>>> fix it in this change but leave the discussion for a different RFR.
>>>
>>> There are a few other changes left in the patch though, so I'm still 
>>> looking for reviewers :-)
>>>
>>> The latest webrev is:
>>> http://cr.openjdk.java.net/~jwilhelm/8026849/webrev.1/
>>>
>>> Thanks,
>>> /Jesper
>>>
>>>
>>> Jesper Wilhelmsson skrev 30/1/14 12:04 PM:
>>>> Hi Kirk,
>>>>
>>>> Your opinion is as important as anyone else's. I have removed the 
>>>> change in
>>>> timer.cpp and updated the webrev. I also closed the bug as WNF.
>>>>
>>>> Cheers,
>>>> /Jesper
>>>>
>>>>
>>>> Kirk Pepperdine skrev 29/1/14 4:22 PM:
>>>>> Hi Jesper,
>>>>>
>>>>> Well, I’m not a reviewer so you can easily ignore my plea for 
>>>>> stability in the
>>>>> GC logs but I appreciate the consideration.
>>>>>
>>>>> Regards,
>>>>> Kirk
>>>>>
>>>>> On Jan 29, 2014, at 3:01 PM, Jesper Wilhelmsson
>>>>> <jesper.wilhelmsson at oracle.com> wrote:
>>>>>
>>>>>> Hi Kirk,
>>>>>>
>>>>>> Thanks for looking at this!
>>>>>>
>>>>>> As I wrote in bug 8023899, I agree with your assessment that it 
>>>>>> may not be
>>>>>> worth the effort to fix the comma. I won't fight for fixing it, I 
>>>>>> just wanted
>>>>>> to get a few official comments as motivation to close the bug as 
>>>>>> WNF. :-)
>>>>>>
>>>>>> Thanks,
>>>>>> /Jesper
>>>>>>
>>>>>>
>>>>>> Kirk Pepperdine skrev 29/1/14 8:54 AM:
>>>>>>>
>>>>>>> On Jan 29, 2014, at 12:29 AM, Jesper Wilhelmsson 
>>>>>>> <jesper.wilhelmsson at oracle.com
>>>>>>> <mailto:jesper.wilhelmsson at oracle.com>> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Could I have a couple of reviews for this small patch?
>>>>>>>>
>>>>>>>> These are fixes for a couple of typos that I didn't include in 
>>>>>>>> the big fix
>>>>>>>> recently pushed. They were not included since they do not 
>>>>>>>> reside in comments
>>>>>>>> but rather assert messages, flag descriptions and some verbose 
>>>>>>>> messages.
>>>>>>>>
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8026849
>>>>>>>>
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~jwilhelm/8026849/webrev/
>>>>>>>>
>>>>>>>>
>>>>>>>> This patch also contains a fix for 8023899 - Typo in 
>>>>>>>> TraceCPUTime message.
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8023899
>>>>>>>
>>>>>>> -        _logfile->print(" [Times: user=%3.2f sys=%3.2f, 
>>>>>>> real=%3.2f secs] ",
>>>>>>> +        _logfile->print(" [Times: user=%3.2f sys=%3.2f 
>>>>>>> real=%3.2f secs] ",
>>>>>>>             user_secs, system_secs, real_secs);
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> caveat, not a official reviewer. I looked at all the typo fixes 
>>>>>>> and they all
>>>>>>> look good. The only thing I can comment on is that I appreciate 
>>>>>>> what you’re
>>>>>>> trying to do here but I don’t really care if there is a comma or 
>>>>>>> a space
>>>>>>> between
>>>>>>> the entries as I don’t read these files but my parser does read 
>>>>>>> them and it
>>>>>>> cares. Yeah it’s a trivial fix to the parser for this one but 
>>>>>>> when I look at
>>>>>>> all
>>>>>>> the spaces, commas, ’s’es and other trivial formatting changes 
>>>>>>> that creep in
>>>>>>> over time it starts to get really messy. Can we please just 
>>>>>>> leave this one out?
>>>>>>>
>>>>>>> Kind regards,
>>>>>>> Kirk
>>>>>>>
>>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20140130/7ec9532d/attachment-0001.html 


More information about the hotspot-runtime-dev mailing list