RFR (s): 8026849 - Fix typos in the GC code, part 2
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Thu Jan 30 14:24:50 UTC 2014
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
>>>>>>
>>>>
>
More information about the hotspot-gc-dev
mailing list