JDK 8 code review request for 7132338 Use @code friendly idiom for '\' in javadoc
Hello, Responding to some of code review feedback from Ulf about the unsigned API work, I've taken a pass at purging <code>'\u0030'</code> from the jdk repo's javadoc and replacing it with {@code '\u005Cu0030'} since {@code} is generally preferable. Webrev with these changes and other <code></code> => {@code} transformations at http://cr.openjdk.java.net/~darcy/7132338.0/ With these changes, the javadoc builds runs without additional warnings and a specdiff against a reference copy of the javadoc shows no unexpected changes. Thanks, -Joe
On 23/01/2012 08:23, Joe Darcy wrote:
Hello,
Responding to some of code review feedback from Ulf about the unsigned API work, I've taken a pass at purging
<code>'\u0030'</code>
from the jdk repo's javadoc and replacing it with
{@code '\u005Cu0030'}
since {@code} is generally preferable.
Webrev with these changes and other <code></code> => {@code} transformations at
http://cr.openjdk.java.net/~darcy/7132338.0/
With these changes, the javadoc builds runs without additional warnings and a specdiff against a reference copy of the javadoc shows no unexpected changes.
Thanks,
-Joe I looked through the patch. Looks like there is an issue in java.io.RandomAccessFile where all <code> usages have been changed to </code>. In java.io.DataInput I see that the javadoc doesn't go beyond about column 40 in many cases and maybe it would be good to clean this up while you are there.
-Alan.
Am 23.01.2012 11:33, schrieb Alan Bateman:
On 23/01/2012 08:23, Joe Darcy wrote:
With these changes, the javadoc builds runs without additional warnings and a specdiff against a reference copy of the javadoc shows no unexpected changes. I'm wondering because of </code>...</code> error, Alan pointed out.
In java.io.DataInput I see that the javadoc doesn't go beyond about column 40 in many cases and maybe it would be good to clean this up while you are there. +1 It would be good, to have a place at jdk repo for such cosmetic scripts (preferably for Unix + Windows). anyone could contribute for enhancement + check the result at areas, one is working on, e.g. to avoid unwanted changes like intentional line breaks outside the column 80 rule.
Joe, thanks for catching my change proposal. Would like to see me as contributor :-) I'm wondering, that you only found 14 classes to change. IIRC, there are at least usages of old <code>...</code> tags in java.lang.Character + java.lang.AbstractStringBuilder. Please refer the 'cosmetic' patches at [1] for additional javadoc + indentation + spacing corrections. I think, it is good style, to have 2 spaces after period '.' in javadoc. [1] https://bugs.openjdk.java.net/show_bug.cgi?id=100104 -Ulf
On 01/23/2012 05:11 AM, Ulf Zibis wrote:
Am 23.01.2012 11:33, schrieb Alan Bateman:
On 23/01/2012 08:23, Joe Darcy wrote:
With these changes, the javadoc builds runs without additional warnings and a specdiff against a reference copy of the javadoc shows no unexpected changes. I'm wondering because of </code>...</code> error, Alan pointed out.
In java.io.DataInput I see that the javadoc doesn't go beyond about column 40 in many cases and maybe it would be good to clean this up while you are there. +1 It would be good, to have a place at jdk repo for such cosmetic scripts (preferably for Unix + Windows). anyone could contribute for enhancement + check the result at areas, one is working on, e.g. to avoid unwanted changes like intentional line breaks outside the column 80 rule.
Well, there is the make/scripts directory in the the top-level JDK repository forest.
Joe, thanks for catching my change proposal. Would like to see me as contributor :-)
I'm wondering, that you only found 14 classes to change. IIRC, there are at least usages of old <code>...</code> tags in java.lang.Character + java.lang.AbstractStringBuilder.
I was not trying to replace all the <code></code> tags with {@code}! Although others may attempt that later in JDK 8. (For the classes I directly maintain, I long ago migrated to using {@code}.) I was only attempting to replace the awkward HTML with <code></code> used to render "\u..." with less awkward javadoc just using {@code}, but if I was in a file, I replaced the other <code></code> usages to. Cheers, -Joe
Am 23.01.2012 18:53, schrieb Joe Darcy:
Well, there is the make/scripts directory in the the top-level JDK repository forest. Could you please publish your script there? (maybe via this patch)
Joe, thanks for catching my change proposal. Would like to see me as contributor :-)
I meant via the "contributed-by" line in the final HG change set.
I was only attempting to replace the awkward HTML with <code></code> used to render "\u..." with less awkward javadoc just using {@code}, but if I was in a file, I replaced the other <code></code> usages to.
Thanks, ok. Was not obvious from the bugs subject line. Additionally I see inconsistent indentations e.g. in AbstractStringBuilder: lines 838-846 lines 1241-1244 vs. 1259-1265 ... and I still think, using 'unsigned {@code int}' in e.g. Integer.java line 628 is not correct, as there is no unsigned int in Java for now. Suggestion: 'unsigned 32-bit integer' -Ulf
On 01/23/2012 10:29 AM, Ulf Zibis wrote:
Am 23.01.2012 18:53, schrieb Joe Darcy:
Well, there is the make/scripts directory in the the top-level JDK repository forest. Could you please publish your script there? (maybe via this patch)
I just used some semi-automated search and replace in emacs so no stand-alone script at this point.
Joe, thanks for catching my change proposal. Would like to see me as contributor :-)
I meant via the "contributed-by" line in the final HG change set.
But you didn't work on generating the patch! Getting credit for filing the bug and getting credit for fixing it are different :-) -Joe
On 01/23/2012 02:33 AM, Alan Bateman wrote:
On 23/01/2012 08:23, Joe Darcy wrote:
Hello,
Responding to some of code review feedback from Ulf about the unsigned API work, I've taken a pass at purging
<code>'\u0030'</code>
from the jdk repo's javadoc and replacing it with
{@code '\u005Cu0030'}
since {@code} is generally preferable.
Webrev with these changes and other <code></code> => {@code} transformations at
http://cr.openjdk.java.net/~darcy/7132338.0/
With these changes, the javadoc builds runs without additional warnings and a specdiff against a reference copy of the javadoc shows no unexpected changes.
Thanks,
-Joe I looked through the patch. Looks like there is an issue in java.io.RandomAccessFile where all <code> usages have been changed to </code>. In java.io.DataInput I see that the javadoc doesn't go beyond about column 40 in many cases and maybe it would be good to clean this up while you are there.
Hi Alan, Not sure what happened in RandomAccessFile; specdiff caught various problems with earlier versions of the work before I send the webrev out for review. In any case, thanks for the catching the problem; an updated webrev with a corrected (but not re-columned) RandomAccessFile is at: http://cr.openjdk.java.net/~darcy/7132338.1/ -Joe
On 23/01/2012 17:41, Joe Darcy wrote:
Hi Alan,
Not sure what happened in RandomAccessFile; specdiff caught various problems with earlier versions of the work before I send the webrev out for review. In any case, thanks for the catching the problem; an updated webrev with a corrected (but not re-columned) RandomAccessFile is at:
Updated webrev looks fine. It would be nice to get java.io.DataInput re-formatted but understand if you won't want to don't want to take it while you are there. -Alan
participants (3)
-
Alan Bateman
-
Joe Darcy
-
Ulf Zibis