Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos jdk.compiler and jdk.javadoc look good - Marked as reviewed by jjg (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Marked as reviewed by egahlin (Reviewer). The JFR related changes looks OK. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Client changes looks good to me. - Marked as reviewed by azvegint (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos javac changes look good to me. - Marked as reviewed by jlahoda (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 11:40:20 GMT, Lance Andersen wrote: >> OK, so lines 264, 295, 329, 364, 431 are arguably wrong as well? >> Separating the [] completely looks quite rare. >> I'll leave it up to you. 8-) > > I think that can be a follow on clean up. The strange formatting of `long []updateCounts` looks very unusual and well worth a followup cleanup. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 10:59:13 GMT, Pavel Rappo wrote: >> src/java.base/share/classes/sun/text/RuleBasedBreakIterator.java line 73: >> >>> 71: * will be transparent to the BreakIterator. A sequence of >>> characters will break the >>> 72: * same way it would if any ignore characters it contains are taken >>> out. Break >>> 73: * positions never occur before ignore characters. >> >> "before ignored characters" > > I believe it's the name of a concept, so I will leave it as is: > >> There is one special substitution. If the description defines a >> substitution called "", the expression must be a [] expression, and >> the expression defines a set of characters (the "ignore characters") that >> will be transparent to the BreakIterator. Yes, that is correct. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 13:48:02 GMT, Pavel Rappo wrote: > > Yes an "or" is probably worthwhile to add. > > I would prefer to leave it for the follow-up cleanup if only because adding > "or" here would make it inconsistent with other `@throws SQLException` > descriptions in that file and perhaps elsewhere in java.sql: > > * java/sql/Statement.java:59 > * java/sql/Statement.java:85 > * java/sql/Statement.java:346 > * java/sql/Statement.java:524 > * java/sql/Statement.java:745 > * java/sql/Statement.java:814 > * java/sql/Statement.java:860 > * java/sql/Statement.java:913 > * java/sql/Statement.java:962 > * java/sql/Statement.java:1225 > * java/sql/Statement.java:1269 > * java/sql/Statement.java:1314 I am fine with that. I guess a semi-colon could also be used vs. a comma to divide the Exception scenarios listed - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 12:37:46 GMT, Pavel Rappo wrote: > > The above is a bit confusing as it reads(same in ImageInputStream.java). I > > think that that can be looked at later. > > Just to clarify: you are okay with removing the ` ` entity, right? As for > ImageInputStream, some doc comments should probably use `@inheritDoc`. But > this is a much bigger clean-up. Yes I am. It just does not flow as well as it could IMHO - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - `` is an apostrophe, which does not require to be encoded. > > Pavel Rappo has updated the pull request incrementally with one additional > commit since the last revision: > > Additional typos Hi Pavel, Looks good. Thanks, Serguei - Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos [v2]
> - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - `` is an apostrophe, which does not require to be encoded. Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision: Additional typos - Changes: - all: https://git.openjdk.java.net/jdk/pull/7063/files - new: https://git.openjdk.java.net/jdk/pull/7063/files/fe81eeca..b256a13f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7063=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7063=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7063.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7063/head:pull/7063 PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:38:13 GMT, Lance Andersen wrote: > Yes an "or" is probably worthwhile to add. I would prefer to leave it for the follow-up cleanup if only because adding "or" here would make it inconsistent with other `@throws SQLException` descriptions in that file and perhaps elsewhere in java.sql: * java/sql/Statement.java:59 * java/sql/Statement.java:85 * java/sql/Statement.java:346 * java/sql/Statement.java:524 * java/sql/Statement.java:745 * java/sql/Statement.java:814 * java/sql/Statement.java:860 * java/sql/Statement.java:913 * java/sql/Statement.java:962 * java/sql/Statement.java:1225 * java/sql/Statement.java:1269 * java/sql/Statement.java:1314 - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - `` is an apostrophe, which does not require to be encoded. The change in the one security class (KeyStoreLoginModule) looks fine. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:42:48 GMT, Lance Andersen wrote: > The above is a bit confusing as it reads(same in ImageInputStream.java). I > think that that can be looked at later. Just to clarify: you are okay with removing the ` ` entity, right? As for ImageInputStream, some doc comments should probably use `@inheritDoc`. But this is a much bigger clean-up. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:29:35 GMT, Pavel Rappo wrote:
>> src/java.sql/share/classes/java/sql/Statement.java line 784:
>>
>>> 782: * statement returns a {@code ResultSet} object, the second
>>> argument
>>> 783: * supplied to this method is not an
>>> 784: * {@code int} array whose elements are valid column indexes, the
>>> method is called on a
>>
>> Should it be "or the method is called on...", i.e. add the "or", otherwise
>> it's a list of problems but we don't know if they are all required, or are
>> alternatives. It probably does not mean that all these have to be true to
>> throw the exception, but it doesn't say that. We are nitpicking of course.
>
> You are right in that this `@throws` description reads a bit weird in its
> current form. That said, I wouldn't touch it in this PR for two reasons.
> Firstly, this wording seems to be consistent with other locations in that
> file. Secondly, this is a spec territory.
Yes an "or" is probably worthwhile to add.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:28:34 GMT, Kevin Walls wrote: >> I thought about it too, but then noticed how the position of `[]` mimics >> that of the respective method signatures in code. > > OK, so lines 264, 295, 329, 364, 431 are arguably wrong as well? Separating > the [] completely looks quite rare. > I'll leave it up to you. 8-) I think that can be a follow on clean up. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote:
> - Most of the typos are of a trivial kind: missing whitespace.
> - If any of the typos should be fixed in the upstream projects instead,
> please say so; I will drop those typos from the patch.
> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant
> formatting artefact and thus can be removed.
> - `` is an apostrophe, which does not require to be encoded.
Overall this looks good, thanks for the clean up :-)
A few comments below
src/java.base/share/classes/java/io/DataInput.java line 496:
> 494: * ceases. If the character {@code '\r'}
> 495: * is encountered, it is discarded and, if
> 496: * the following byte converts to the
The above is a bit confusing as it reads(same in ImageInputStream.java). I
think that that can be looked at later.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:18:42 GMT, Kevin Walls wrote:
>> - Most of the typos are of a trivial kind: missing whitespace.
>> - If any of the typos should be fixed in the upstream projects instead,
>> please say so; I will drop those typos from the patch.
>> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant
>> formatting artefact and thus can be removed.
>> - `` is an apostrophe, which does not require to be encoded.
>
> src/java.sql/share/classes/java/sql/Statement.java line 784:
>
>> 782: * statement returns a {@code ResultSet} object, the second argument
>> 783: * supplied to this method is not an
>> 784: * {@code int} array whose elements are valid column indexes, the
>> method is called on a
>
> Should it be "or the method is called on...", i.e. add the "or", otherwise
> it's a list of problems but we don't know if they are all required, or are
> alternatives. It probably does not mean that all these have to be true to
> throw the exception, but it doesn't say that. We are nitpicking of course.
You are right in that this `@throws` description reads a bit weird in its
current form. That said, I wouldn't touch it in this PR for two reasons.
Firstly, this wording seems to be consistent with other locations in that file.
Secondly, this is a spec territory.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - `` is an apostrophe, which does not require to be encoded. Marked as reviewed by kevinw (Committer). - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:04:43 GMT, Pavel Rappo wrote:
>> src/java.sql/share/classes/java/sql/BatchUpdateException.java line 58:
>>
>>> 56: * A JDBC driver implementation should use
>>> 57: * the constructor {@code BatchUpdateException(String reason, String
>>> SQLState,
>>> 58: * int vendorCode, long []updateCounts, Throwable cause) } instead of
>>
>> While we are here, is it more normal to say "long[] updateCounts", not
>> separating the [] from the type.
>> Similarly at line 81, 118, 151, 247, 277, 318, 354.
>
> I thought about it too, but then noticed how the position of `[]` mimics that
> of the respective method signatures in code.
OK, so lines 264, 295, 329, 364, 431 are arguably wrong as well? Separating
the [] completely looks quite rare.
I'll leave it up to you. 8-)
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote:
> - Most of the typos are of a trivial kind: missing whitespace.
> - If any of the typos should be fixed in the upstream projects instead,
> please say so; I will drop those typos from the patch.
> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant
> formatting artefact and thus can be removed.
> - `` is an apostrophe, which does not require to be encoded.
src/java.sql/share/classes/java/sql/Statement.java line 784:
> 782: * statement returns a {@code ResultSet} object, the second argument
> 783: * supplied to this method is not an
> 784: * {@code int} array whose elements are valid column indexes, the
> method is called on a
Should it be "or the method is called on...", i.e. add the "or", otherwise it's
a list of problems but we don't know if they are all required, or are
alternatives. It probably does not mean that all these have to be true to
throw the exception, but it doesn't say that. We are nitpicking of course.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 11:00:55 GMT, Kevin Walls wrote:
>> - Most of the typos are of a trivial kind: missing whitespace.
>> - If any of the typos should be fixed in the upstream projects instead,
>> please say so; I will drop those typos from the patch.
>> - As I understand it, in ImageInputStream and DataInput is an irrelevant
>> formatting artefact and thus can be removed.
>> - is an apostrophe, which does not require to be encoded.
>
> src/java.sql/share/classes/java/sql/BatchUpdateException.java line 58:
>
>> 56: * A JDBC driver implementation should use
>> 57: * the constructor {@code BatchUpdateException(String reason, String
>> SQLState,
>> 58: * int vendorCode, long []updateCounts, Throwable cause) } instead of
>
> While we are here, is it more normal to say "long[] updateCounts", not
> separating the [] from the type.
> Similarly at line 81, 118, 151, 247, 277, 318, 354.
I thought about it too, but then noticed how the position of `[]` mimics that
of the respective method signatures in code.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:46:11 GMT, Kevin Walls wrote: >> - Most of the typos are of a trivial kind: missing whitespace. >> - If any of the typos should be fixed in the upstream projects instead, >> please say so; I will drop those typos from the patch. >> - As I understand it, in ImageInputStream and DataInput is an irrelevant >> formatting artefact and thus can be removed. >> - is an apostrophe, which does not require to be encoded. > > src/java.base/share/classes/sun/text/RuleBasedBreakIterator.java line 73: > >> 71: * will be transparent to the BreakIterator. A sequence of >> characters will break the >> 72: * same way it would if any ignore characters it contains are taken >> out. Break >> 73: * positions never occur before ignore characters. > > "before ignored characters" I believe it's the name of a concept, so I will leave it as is: > There is one special substitution. If the description defines a substitution > called "", the expression must be a [] expression, and the expression > defines a set of characters (the "ignore characters") that will be > transparent to the BreakIterator. - PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote:
> - Most of the typos are of a trivial kind: missing whitespace.
> - If any of the typos should be fixed in the upstream projects instead,
> please say so; I will drop those typos from the patch.
> - As I understand it, in ImageInputStream and DataInput is an irrelevant
> formatting artefact and thus can be removed.
> - is an apostrophe, which does not require to be encoded.
src/java.sql/share/classes/java/sql/BatchUpdateException.java line 58:
> 56: * A JDBC driver implementation should use
> 57: * the constructor {@code BatchUpdateException(String reason, String
> SQLState,
> 58: * int vendorCode, long []updateCounts, Throwable cause) } instead of
While we are here, is it more normal to say "long[] updateCounts", not
separating the [] from the type.
Similarly at line 81, 118, 151, 247, 277, 318, 354.
-
PR: https://git.openjdk.java.net/jdk/pull/7063
Re: RFR: 8279918: Fix various doc typos
On Thu, 13 Jan 2022 10:30:07 GMT, Pavel Rappo wrote: > - Most of the typos are of a trivial kind: missing whitespace. > - If any of the typos should be fixed in the upstream projects instead, > please say so; I will drop those typos from the patch. > - As I understand it, in ImageInputStream and DataInput is an irrelevant > formatting artefact and thus can be removed. > - is an apostrophe, which does not require to be encoded. src/java.base/share/classes/sun/text/RuleBasedBreakIterator.java line 73: > 71: * will be transparent to the BreakIterator. A sequence of > characters will break the > 72: * same way it would if any ignore characters it contains are taken > out. Break > 73: * positions never occur before ignore characters. "before ignored characters" - PR: https://git.openjdk.java.net/jdk/pull/7063
