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