Re: RFR: 8325109: Sort method modifiers in canonical order
On Thu, 1 Feb 2024 11:57:04 GMT, Magnus Ihse Bursie wrote: > This is a follow-up on > [JDK-8324053](https://bugs.openjdk.org/browse/JDK-8324053). I have run the > bin/blessed-modifier-order.sh on the entire code base, and manually checked > the result. I have reverted all but these trivial and uncontroversial changes. > > Almost all of these are about moving `static` to its proper position; a few > do not involve `static` but instead puts `final` or `abstract` in the correct > place. > > I have deliberately stayed away from `default` in this PR, since they should > probably get a more thorough treatment, see > https://github.com/openjdk/jdk/pull/17468#pullrequestreview-1829238473. Looks good to me. I looked through all the changes. - Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17670#pullrequestreview-1856683827
Re: RFR: JDK-8322782: Clean up usages of unnecessary fully qualified class name "java.util.Arrays" [v3]
On Wed, 3 Jan 2024 13:55:22 GMT, Matthias Baesken wrote: >> In [JDK-8322772](https://bugs.openjdk.org/browse/JDK-8322772) one similar >> cleanup has been proposed before (and was done in the change). But there are >> a number of other places in the codebase where the import is done and still >> the unneeded fully qualified class name "java.util.Arrays" is used so more >> cleanups can be done. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > Adjust Invokers.java too Looks good to me. - Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17241#pullrequestreview-1802684965
Re: RFR: JDK-8315897: some PrivilegedActions missing in JDK code for getting properties
On Fri, 8 Sep 2023 13:02:07 GMT, Alan Bateman wrote: > So what about FontConfiguration? Is that something using the class directly > too? I think this is not needed either. As far as I can see, the instance of `FontConfiguration` is created from `doPrivileged`, therefore these two system properties are read inside `doPrivileged` already. - PR Comment: https://git.openjdk.org/jdk/pull/15629#issuecomment-1711722806
Re: RFR: 8267174: Many test files have the wrong Copyright header
On Tue, 5 Sep 2023 22:49:41 GMT, Erik Joelsson wrote: > There are a number of files in the `test` directory that have an incorrect > copyright header, which includes the "classpath" exception text. This patch > removes that text from all test files that I could find it in. I did this > using a combination of `sed` and `grep`. Reviewing this patch is probably > easier using the raw patch file or a suitable webrev format. > > It's my assumption that these headers were introduced by mistake as it's > quite easy to copy the wrong template when creating new files. Client changes look good. I've looked through all the files, other files look good too. - Marked as reviewed by aivanov (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15573#pullrequestreview-1613295743
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments
On Fri, 3 Mar 2023 10:09:27 GMT, Claes Redestad wrote: > Yes, iff means if-and-only-if and is used for extra precision in formal > logic, mathematics. I've never come across it before. With your explanations, it makes perfect sense. - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments
On Thu, 2 Mar 2023 12:03:44 GMT, Pavel Rappo wrote:
> Please review this superficial documentation cleanup that was triggered by
> unrelated analysis of doc comments in JDK API.
>
> The only effect that this multi-area PR has on the JDK API Documentation
> (i.e. the observable effect on the generated HTML pages) can be summarized as
> follows:
>
>
> diff -ur build/macosx-aarch64/images/docs-before/api/serialized-form.html
> build/macosx-aarch64/images/docs-after/api/serialized-form.html
> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html
> 2023-03-02 11:47:44
> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html
> 2023-03-02 11:48:45
> @@ -17084,7 +17084,7 @@
> throws href="java.base/java/io/IOException.html" title="class in
> java.io">IOException,
> ClassNotFoundException
> readObject is called to restore the
> state of the
> - (@code BasicPermission} from a stream.
> + BasicPermission from a stream.
>
> Parameters:
> s - the ObjectInputStream from which data
> is read
>
> Notes
> -
>
> * I'm not an expert in any of the affected areas, except for jdk.javadoc, and
> I was merely after misused tags. Because of that, I would appreciate reviews
> from experts in other areas.
> * I discovered many more issues than I included in this PR. The excluded
> issues seem to occur in infrequently updated third-party code (e.g.
> javax.xml), which I assume we shouldn't touch unless necessary.
> * I will update copyright years after (and if) the fix had been approved, as
> required.
Looks good to me.
I looked through all the changes, paying more attention to the client area.
src/java.base/share/classes/java/lang/invoke/BootstrapMethodInvoker.java line
257:
> 255:
> 256: /**
> 257: * @return true iff the BSM method type exactly matches
I assume “iff” should “if”?
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 2866:
> 2864: * Merge multiple abstract methods. The preferred method is a
> method that is a subsignature
> 2865: * of all the other signatures and whose return type is more
> specific {@link MostSpecificReturnCheck}.
> 2866: * The resulting preferred method has a thrown clause that is the
> intersection of the merged
Is it “…has a {@code throws} clause…”?
-
Marked as reviewed by aivanov (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8301659: Resolve initialization reordering issues on Windows for libawt and libsaproc
On Thu, 2 Feb 2023 06:12:20 GMT, Julian Waters wrote: > Small, trivial change to resolve initialization order reordering in > constructors, required for > [JDK-8301659](https://bugs.openjdk.org/browse/JDK-8301659) Looks good to me. David's comment sheds some light on why it's needed. I will appreciate if you add more details though. - Marked as reviewed by aivanov (Reviewer). PR: https://git.openjdk.org/jdk/pull/12382
Re: RFR: 8301659: Resolve initialization reordering issues on Windows for libawt and libsaproc
On Thu, 2 Feb 2023 12:12:37 GMT, Kevin Walls wrote: > Could we say in the bug exactly when this is an issue (maybe it's a certain > compiler?), and include a copy of the error or warning that is seen? Yes, I agree. The change is simple enough yet there are no details why it's needed. - PR: https://git.openjdk.org/jdk/pull/12382
Re: RFR: 8299563: Fix typos [v4]
On Wed, 4 Jan 2023 16:35:41 GMT, Michael Ernst wrote: >> 8299563: Fix typos > > Michael Ernst has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains ten commits: > > - Address review feedback > - Merge ../jdk-openjdk into typos-typos > - Merge ../jdk-openjdk into typos-typos > - Reinstate typos in Apache code that is copied into the JDK > - Merge ../jdk-openjdk into typos-typos > - Remove file that was removed upstream > - Fix inconsistency in capitalization > - Undo change in zlip > - Fix typos Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.org/jdk/pull/10029
Re: RFR: 8295729: Add jcheck whitespace checking for properties files [v3]
On Mon, 24 Oct 2022 19:21:07 GMT, Magnus Ihse Bursie wrote: >> Properties files is essentially source code. It should have the same >> whitespace checks as all other source code, so we don't get spurious >> trailing whitespace changes. >> >> With the new Skara jcheck, it is possible to increase the coverage of the >> whitespace checks (in the old mercurial version, this was more or less >> impossible). >> >> The only manual change is to `.jcheck/conf`. All other changes were made by >> running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ >> \t]*$//'`. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Revert "Remove check for .properties from jcheck" > >This reverts commit c91fdaa19dc06351598bd1c0614e1af3bfa08ae2. > - Change trailing space and tab in values to unicode encoding Trailing spaces in `LocaleNames_*` are only in two files: - `src/jdk.localedata/share/classes/sun/util/resources/ext/LocaleNames_de.properties` - `src/jdk.localedata/share/classes/sun/util/resources/ext/LocaleNames_sv.properties` It is very unlikely these spaces are part of a country or language name. The former file contains a few trailing spaces, the latter — only one. src/jdk.localedata/share/classes/sun/util/resources/ext/LocaleNames_de.properties line 238: > 236: cpp=Kreolisch-Portugiesische Sprache > 237: crh=Krimtatarisch > 238: crp=Kreolische Sprache\u0020 I'm pretty sure locale names shouldn't contain trailing spaces. - Marked as reviewed by aivanov (Reviewer). PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni [v2]
On Mon, 26 Sep 2022 16:51:36 GMT, Michael Ernst wrote: >> 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni > > Michael Ernst has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - Reinstate typos in Apache code that is copied into the JDK > - Merge ../jdk-openjdk into typos-typos > - Remove file that was removed upstream > - Fix inconsistency in capitalization > - Undo change in zlip > - Fix typos I agree with everyone who said the PR should be broken to smaller pieces so that it touches code / tests in one or two packages, modules. It would be easier to review, you would need to get an approval from reviewers in a one or two specific areas. At this time, this PR touches files in 11 areas according the number of labels which correspond to a specific mailing list where discussions for the area are held. - PR: https://git.openjdk.org/jdk/pull/10029
Re: RFR: 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni [v2]
On Mon, 26 Sep 2022 16:51:36 GMT, Michael Ernst wrote: >> 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni > > Michael Ernst has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - Reinstate typos in Apache code that is copied into the JDK > - Merge ../jdk-openjdk into typos-typos > - Remove file that was removed upstream > - Fix inconsistency in capitalization > - Undo change in zlip > - Fix typos Changes requested by aivanov (Reviewer). src/hotspot/share/opto/memnode.cpp line 2365: > 2363: if (x != this) return x; > 2364: > 2365: // Take apart the address into an oop and offset. “and _an_ offset”? src/java.xml/share/classes/org/w3c/dom/Document.java line 293: > 291: * systemId, and notationName attributes > are > 292: * copied. If a deep import is requested, the > descendants > 293: * of the source Entity are recursively imported and This class may come from a 3rd party library. Anyone from `java.xml` can confirm it? test/hotspot/jtreg/vmTestbase/nsk/share/locks/DeadlockMaker.java line 31: > 29: /* > 30: * Class used to create deadlocked threads. It is possible create 2 or > more deadlocked thread, also > 31: * is possible to specify resource of which type should lock each > deadlocked thread Suggestion: * it is possible to specify resource of which type should lock each deadlocked thread It doesn't sound right without _“it”_. test/jdk/com/sun/jdi/connect/spi/GeneratedConnectors.java line 28: > 26: * @summary Unit test for "Pluggable Connectors and Transports" feature. > 27: * > 28: * When a transport service is deployed the virtual machine Suggestion: * When a transport service is deployed, the virtual machine Let's add a comma for clarity. test/jdk/java/security/testlibrary/SimpleOCSPServer.java line 445: > 443: > 444: /** > 445: * Check the status database for revocation information on one or > more Suggestion: * Check the status database for revocation information of one or more test/jdk/sun/jvmstat/testlibrary/utils.sh line 181: > 179: if [ $? -eq 0 ] > 180: then > 181: # it's still lingering, now it is hard Suggestion: # it's still lingering, now hit it hard - PR: https://git.openjdk.org/jdk/pull/10029
Re: RFR: 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni [v2]
On Mon, 26 Sep 2022 16:51:36 GMT, Michael Ernst wrote: >> 8294321: Fix typos in files under test/jdk/java, test/jdk/jdk, test/jdk/jni > > Michael Ernst has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - Reinstate typos in Apache code that is copied into the JDK > - Merge ../jdk-openjdk into typos-typos > - Remove file that was removed upstream > - Fix inconsistency in capitalization > - Undo change in zlip > - Fix typos The title says under `test/jdk/*`, yet there are lots of files changed in `src/*`. - PR: https://git.openjdk.org/jdk/pull/10029
