Re: RFR: 8285485: Fix typos in corelibs [v4]
> I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/8364/files - new: https://git.openjdk.java.net/jdk/pull/8364/files/8bc35edb..7dc7f653 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8364=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8364=02-03 Stats: 86 lines in 86 files changed: 0 ins; 0 del; 86 mod Patch: https://git.openjdk.java.net/jdk/pull/8364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8364/head:pull/8364 PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs [v3]
> I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Merge branch 'master' into typos-in-corelibs - Fixes comments in review: * Reverting changes in jdwp.spec * Fix "is occurs" * Stop providing "Syncchronication" providers :-) * Fix fEncoder reduplication. * Revert changes in XMLEventReaderImpl.java - Pass #2 core - Pass #1 core - Pass #2 misc - Pass #1 misc - Changes: - all: https://git.openjdk.java.net/jdk/pull/8364/files - new: https://git.openjdk.java.net/jdk/pull/8364/files/b6c00de9..8bc35edb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8364=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8364=01-02 Stats: 228368 lines in 2830 files changed: 173822 ins; 38520 del; 16026 mod Patch: https://git.openjdk.java.net/jdk/pull/8364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8364/head:pull/8364 PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs [v3]
On Wed, 27 Apr 2022 15:50:20 GMT, Roger Riggs wrote: >> Magnus Ihse Bursie has updated the pull request with a new target base due >> to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains six >> additional commits since the last revision: >> >> - Merge branch 'master' into typos-in-corelibs >> - Fixes comments in review: >> >>* Reverting changes in jdwp.spec >>* Fix "is occurs" >>* Stop providing "Syncchronication" providers :-) >>* Fix fEncoder reduplication. >>* Revert changes in XMLEventReaderImpl.java >> - Pass #2 core >> - Pass #1 core >> - Pass #2 misc >> - Pass #1 misc > > Since you've changed some code; you need to run tests for tiers 1-3. > > (Note that even for trivial changes, hundreds of OpenJDK developers are > notified and distracted; be considerate of other developers). I believe I have addressed all feedback in the review, with the possible exception of 3rd party source. @RogerRiggs Are there more 3rd party files I have inadvertently modified, apart from `XMLEventReaderImpl.java`? - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs [v2]
> I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fixes comments in review: * Reverting changes in jdwp.spec * Fix "is occurs" * Stop providing "Syncchronication" providers :-) * Fix fEncoder reduplication. * Revert changes in XMLEventReaderImpl.java - Changes: - all: https://git.openjdk.java.net/jdk/pull/8364/files - new: https://git.openjdk.java.net/jdk/pull/8364/files/d4266e0a..b6c00de9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8364=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8364=00-01 Stats: 13 lines in 6 files changed: 0 ins; 0 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/8364.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8364/head:pull/8364 PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 17:11:34 GMT, Pavel Rappo wrote:
>> I ran `codespell` on modules owned by core-libs, and accepted those changes
>> where it indeed discovered real typos.
>>
>> I will update copyright years using a script before pushing (otherwise like
>> every second change would be a copyright update, making reviewing much
>> harder).
>>
>> The long term goal here is to make tooling support for running `codespell`.
>> The trouble with automating this is of course all false positives. But
>> before even trying to solve that issue, all true positives must be fixed.
>> Hence this PR.
>
> src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLDOMWriterImpl.java
> line 238:
>
>> 236:
>> 237: /**
>> 238: * Creates a DOM Attribute @see org.w3c.dom.Node and associates it
>> with the current DOM element @see org.w3c.dom.Node.
>
> Not that it matters in this PR, but I think I should mention that `@see` tags
> do not work like that. (No action needed from you.)
Yeah. There's a lot of crappy code hanging around. :( Feel free to open a bug
on checking incorrect usage of `@see` tags in the JDK. I'm sure you'll find
more cases than those.
> src/java.xml/share/classes/javax/xml/transform/Transformer.java line 127:
>
>> 125: * namespace URI in curly braces ({}).
>> 126: * @param value The value object. This can be any valid Java
>> object. It is
>> 127: * up to the processor to provide the proper object coersion or to
>> simply
>
> That made me pause: some systems have the notion of _type coercion_; but your
> change looks right.
I also had to read this a couple of time, and check the source.
-
PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 15:45:12 GMT, Roger Riggs wrote:
>> I ran `codespell` on modules owned by core-libs, and accepted those changes
>> where it indeed discovered real typos.
>>
>> I will update copyright years using a script before pushing (otherwise like
>> every second change would be a copyright update, making reviewing much
>> harder).
>>
>> The long term goal here is to make tooling support for running `codespell`.
>> The trouble with automating this is of course all false positives. But
>> before even trying to solve that issue, all true positives must be fixed.
>> Hence this PR.
>
> src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.java
> line 140:
>
>> 138: } else if(type == XMLEvent.START_ELEMENT) {
>> 139: throw new XMLStreamException(
>> 140: "elementGetText() function expects text only element
>> but START_ELEMENT was encountered.", event.getLocation());
>
> Should we be fixing typos in third party software; it can easily get wiped
> out in an update.
No, we should not. Such changes ideally should be brought upstream, but I can't
be bothered. :-(
I'm reverting all changes in this file. Are there any more files that are 3rd
party in this fix? I have no way to tell.
-
PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 16:50:30 GMT, Pavel Rappo wrote:
>> I ran `codespell` on modules owned by core-libs, and accepted those changes
>> where it indeed discovered real typos.
>>
>> I will update copyright years using a script before pushing (otherwise like
>> every second change would be a copyright update, making reviewing much
>> harder).
>>
>> The long term goal here is to make tooling support for running `codespell`.
>> The trouble with automating this is of course all false positives. But
>> before even trying to solve that issue, all true positives must be fixed.
>> Hence this PR.
>
> src/java.se/share/data/jdwp/jdwp.spec line 47:
>
>> 45: "Returns reference types for all the classes loaded by the
>> target VM "
>> 46: "which match the given signature. "
>> 47: "Multiple reference types will be returned if two or more class "
>
> This file's name scares me. The fact that there's no "serviceability" label
> on this PR adds to this feeling. I would ask around if I were you.
I'll revert all changes in this file. I don't want it to hold up the entire PR.
We can revisit it later, with serviceability.
> src/java.sql.rowset/share/classes/com/sun/rowset/JoinRowSetImpl.java line 636:
>
>> 634: // to be INNER JOIN'ED to form a new rowset
>> 635: // Compare table1.MatchColumn1.value1 == {
>> table2.MatchColumn2.value1
>> 636: // ... up to
>> table2.MatchColumn2.valueN }
>
> Curious: it is not some established string representation, is it?
You mean writing "upto" instead of "up to"? No, it's just incorrect, and a
somewhat common typo.
-
PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 16:18:15 GMT, Naoto Sato wrote: >> src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties >> line 188: >> >>> 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757 >>> 187: >>> 188: main.plugin.category=\u7C7B\u522B >> >> what's this? > > Removing the trailing space? A similar one is in the `ja` resource bundle. This happens as a combination of jcheck not enforcing eol space checks on .properties files, and my editor being set to always remove space at eol. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 17:02:40 GMT, Pavel Rappo wrote: >> I ran `codespell` on modules owned by core-libs, and accepted those changes >> where it indeed discovered real typos. >> >> I will update copyright years using a script before pushing (otherwise like >> every second change would be a copyright update, making reviewing much >> harder). >> >> The long term goal here is to make tooling support for running `codespell`. >> The trouble with automating this is of course all false positives. But >> before even trying to solve that issue, all true positives must be fixed. >> Hence this PR. > > src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java > line 239: > >> 237: >> 238: /** >> 239: * Returns the vendor name of the Reference Implementation >> Optimistic > > L240: an optimistic _what_ provider? :-) Yep typo there. The original authors needed to run spell check ;-) - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Changes look OK with the exception of the comment pointed out below - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote:
> I ran `codespell` on modules owned by core-libs, and accepted those changes
> where it indeed discovered real typos.
>
> I will update copyright years using a script before pushing (otherwise like
> every second change would be a copyright update, making reviewing much
> harder).
>
> The long term goal here is to make tooling support for running `codespell`.
> The trouble with automating this is of course all false positives. But before
> even trying to solve that issue, all true positives must be fixed. Hence this
> PR.
Thanks for doing this so carefully. If reviewers decide that parts of this PR
need to be addressed upstream, we should probably consider contributing those
parts to respective projects.
src/java.sql.rowset/share/classes/com/sun/rowset/CachedRowSetImpl.java line 970:
> 968: * and sends a rowSetChanged event to all registered
> 969: * listeners.
> 970: * @throws SQLException if an error is occurs rolling back the RowSet
L976, same as above: "is occurs".
src/java.sql.rowset/share/classes/com/sun/rowset/JoinRowSetImpl.java line 636:
> 634: // to be INNER JOIN'ED to form a new rowset
> 635: // Compare table1.MatchColumn1.value1 == {
> table2.MatchColumn2.value1
> 636: // ... up to
> table2.MatchColumn2.valueN }
Curious: it is not some established string representation, is it?
src/java.sql.rowset/share/classes/com/sun/rowset/internal/SyncResolverImpl.java
line 664:
> 662: * and sends a {@code rowSetChanged} event to all registered
> 663: * listeners.
> 664: * @throws SQLException if an error is occurs rolling back the RowSet
L664: delete "is" in "is occurs".
src/java.sql.rowset/share/classes/com/sun/rowset/providers/RIXMLProvider.java
line 239:
> 237:
> 238: /**
> 239: * Returns the vendor name of the Reference Implementation Optimistic
L240: an optimistic _what_ provider? :-)
src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLDOMWriterImpl.java
line 238:
> 236:
> 237: /**
> 238: * Creates a DOM Attribute @see org.w3c.dom.Node and associates it
> with the current DOM element @see org.w3c.dom.Node.
Not that it matters in this PR, but I think I should mention that `@see` tags
do not work like that. (No action needed from you.)
src/java.xml/share/classes/javax/xml/transform/Transformer.java line 127:
> 125: * namespace URI in curly braces ({}).
> 126: * @param value The value object. This can be any valid Java
> object. It is
> 127: * up to the processor to provide the proper object coersion or to
> simply
That made me pause: some systems have the notion of _type coercion_; but your
change looks right.
-
PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. src/java.se/share/data/jdwp/jdwp.spec line 47: > 45: "Returns reference types for all the classes loaded by the target > VM " > 46: "which match the given signature. " > 47: "Multiple reference types will be returned if two or more class " This file's name scares me. The fact that there's no "serviceability" label on this PR adds to this feeling. I would ask around if I were you. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Wed, 27 Apr 2022 06:39:30 GMT, Athijegannathan Sundararajan wrote: >> I ran `codespell` on modules owned by core-libs, and accepted those changes >> where it indeed discovered real typos. >> >> I will update copyright years using a script before pushing (otherwise like >> every second change would be a copyright update, making reviewing much >> harder). >> >> The long term goal here is to make tooling support for running `codespell`. >> The trouble with automating this is of course all false positives. But >> before even trying to solve that issue, all true positives must be fixed. >> Hence this PR. > > src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties > line 188: > >> 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757 >> 187: >> 188: main.plugin.category=\u7C7B\u522B > > what's this? Removing the trailing space? A similar one is in the `ja` resource bundle. - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote:
> I ran `codespell` on modules owned by core-libs, and accepted those changes
> where it indeed discovered real typos.
>
> I will update copyright years using a script before pushing (otherwise like
> every second change would be a copyright update, making reviewing much
> harder).
>
> The long term goal here is to make tooling support for running `codespell`.
> The trouble with automating this is of course all false positives. But before
> even trying to solve that issue, all true positives must be fixed. Hence this
> PR.
Since you've changed some code; you need to run tests for tiers 1-3.
(Note that even for trivial changes, hundreds of OpenJDK developers are
notified and distracted; be considerate of other developers).
src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEventReaderImpl.java
line 140:
> 138: } else if(type == XMLEvent.START_ELEMENT) {
> 139: throw new XMLStreamException(
> 140: "elementGetText() function expects text only element but
> START_ELEMENT was encountered.", event.getLocation());
Should we be fixing typos in third party software; it can easily get wiped out
in an update.
src/java.xml/share/classes/com/sun/xml/internal/stream/writers/WriterUtility.java
line 97:
> 95: }
> 96: else{
> 97: //attempt to retrieve default fEncoderoder
"fEncoderoder" looks out of place, please recheck.
-
PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. LGTM src/jdk.jlink/share/classes/jdk/tools/jlink/resources/plugins_zh_CN.properties line 188: > 186: main.plugin.module=\u63D2\u4EF6\u6A21\u5757 > 187: > 188: main.plugin.category=\u7C7B\u522B what's this? - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8364
Re: RFR: 8285485: Fix typos in corelibs
On Fri, 22 Apr 2022 15:08:51 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on modules owned by core-libs, and accepted those changes > where it indeed discovered real typos. > > I will update copyright years using a script before pushing (otherwise like > every second change would be a copyright update, making reviewing much > harder). > > The long term goal here is to make tooling support for running `codespell`. > The trouble with automating this is of course all false positives. But before > even trying to solve that issue, all true positives must be fixed. Hence this > PR. Looks fine to me. - Marked as reviewed by jpai (Committer). PR: https://git.openjdk.java.net/jdk/pull/8364
