<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Pavel,</p>
<p>Thanks for looking at this. It was worth reading if only for the
occasional amusement at some of the typos, and occasional guilt at
recognizing some that I probably made. Here are some notes; none
need a followup webrev.</p>
<p>I've reviewed all the individual changes, but since neither of us
work routinely on all the javac source code, I would recommend
also getting buy-in from an active javac developer that a
widespread update like this is acceptable.</p>
<p>-- Jon<br>
</p>
<p><br>
</p>
<p>In
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java,<br>
you missed a typo ;-)Â "influences" should be "influenced"</p>
<p>In Pretty.java, the guideline is to avoid use of XXX and use
something like XYZ instead.<span class="removed" style="color:
brown;"></span><br class="Apple-interchange-newline">
<span class="removed" style="color: brown;"></span></p>
<pre style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; background-color: rgb(238, 238, 238); text-decoration: none;"><span class="removed" style="color: brown;">- /** Exception to propogate IOException through visitXXX methods */</span>
<span class="new" style="color: blue; font-weight: normal;">+ /** Exception to propagate IOException through visitXXX methods */</span></pre>
<div class="moz-cite-prefix">In sjavac Source.java, "lets" should be
"let's"<br>
</div>
<div class="moz-cite-prefix">
<pre style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; background-color: rgb(238, 238, 238); text-decoration: none;"><span class="removed" style="color: brown;">
- // Oups, the source is already added, could be ok, could be not, lets check.</span>
<span class="new" style="color: blue; font-weight: normal;">+ // Oops, the source is already added, could be ok, could be not, lets check.</span></pre>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">-- Jon</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 12/20/19 7:44 AM, Pavel Rappo wrote:<br>
</div>
<blockquote type="cite"
cite="mid:26572007-A4D1-4230-ACAE-8ECAE2F5FE86@oracle.com">
<pre class="moz-quote-pre" wrap="">Hello,
Please review the following change for <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8236435">https://bugs.openjdk.java.net/browse/JDK-8236435</a>:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~prappo/8236435/webrev.00/">http://cr.openjdk.java.net/~prappo/8236435/webrev.00/</a>
I've been reading up on the javadoc code and comments. At one point, jumping
through the links, I reached the javax.tools.DocumentationTool and
com.sun.tools.javac.util.ClientCodeException, which reside in java.compiler and
jdk.compiler respectively. I found a couple of typos there and fixed them.
One thing led to another and half an hour later I found myself compulsively
checking the full javac code base for typos and spelling. Sigh.
Long story short, I just went ahead and fixed as many issues as I could in one
sitting. There was a lot.
On the bright side, this patch doesn't involve any code changes (except for
spelling in internal variables and methods' parameters), addresses aesthetic
issues, readability, helps with searches and assists IDEs to pick up usages
more reliably when refactoring (e.g. renaming).
The initial concern with DocumentationTool turned out to be more subtle and I
decided not to include it in this patch. It is definitely not just a typo and
might require a CSR.
Please review at your convenience. Hopefully, if there are false positives,
you'll catch them.
I suggest reviewing this change using a diff tool with a character-level
resolution. That is, a tool capable of highlighting mismatching characters in
lines that differ.
All test/langtools tests pass. Copyright years will be fixed before the push.
Thanks,
-Pavel
</pre>
</blockquote>
</body>
</html>