Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v4]
On Fri, 19 Mar 2021 18:41:51 GMT, Roger Riggs wrote: >> Intermittent failures on Windows in a test of destroying the child warrant >> extending the time the parent waits after starting the child before >> destroying the child. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify modification of argument list after creating ProcessBuilder Looks good to me, if the intent was to fix potential write blockages in the child process (in which case maybe reformulate the issue title). I'm sorry I derailed your original wait idea, as well as Ioi's ideas of syncing with the child. Both may still be needed after this fix. Cheers, Thomas - Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3049
Integrated: 8263890: Broken links to Unicode.org
On Fri, 19 Mar 2021 17:57:31 GMT, Naoto Sato wrote: > Fixed several broken links to Unicode.org. This pull request has now been integrated. Changeset: 96e5c3f1 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/96e5c3f1 Stats: 37 lines in 8 files changed: 4 ins; 0 del; 33 mod 8263890: Broken links to Unicode.org Reviewed-by: redestad, joehw, iris - PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]
On Sun, 14 Mar 2021 12:57:01 GMT, Alan Bateman wrote: >> Philippe Marschall has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 15 commits: >> >> - Merge master >> - Fix bug in CharArrayReader and add unit test >> - Clean up unit tests >> - Revert off-heap code path >> - Replace c-style array declarations >> - Share work buffer between #skip and #read >> - Update copyright year >> - Implement review comment >> - Revert StreamDecoder changes >> - Implement CharArrayReader#read(CharBuffer) >> - ... and 5 more: >> https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0 > > src/java.base/share/classes/java/io/Reader.java line 205: > >> 203: target.put(cbuf, 0, nread); >> 204: } >> 205: return nread; > > Thanks for bringing this back to just the heap buffer case. This part looks > good now. Agreed. - PR: https://git.openjdk.java.net/jdk/pull/1915
Re: RFR: 8263890: Broken links to Unicode.org [v2]
On Fri, 19 Mar 2021 21:23:03 GMT, Naoto Sato wrote: >> Fixed several broken links to Unicode.org. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments. Looks all good. Thanks Naoto. - Marked as reviewed by joehw (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 8263890: Broken links to Unicode.org [v2]
On Fri, 19 Mar 2021 18:43:30 GMT, Joe Wang wrote: > Some minor comments. Thanks, Joe. Addressed them as suggested. - PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 8263890: Broken links to Unicode.org [v2]
> Fixed several broken links to Unicode.org. Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Addressed review comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3093/files - new: https://git.openjdk.java.net/jdk/pull/3093/files/2b32b86a..37093a4d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3093=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3093=00-01 Stats: 24 lines in 6 files changed: 4 ins; 0 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/3093.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3093/head:pull/3093 PR: https://git.openjdk.java.net/jdk/pull/3093
Integrated: 8263892: More modifier order fixes in java.base
On Fri, 19 Mar 2021 18:23:00 GMT, Alex Blewitt wrote: > Additional changes found in `java.base` of `final private` -> `private > final`. Filed with existing bug because it's the same module; can change to a > different bug number if required. This pull request has now been integrated. Changeset: 77ebc110 Author:Alex Blewitt Committer: Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/77ebc110 Stats: 19 lines in 5 files changed: 0 ins; 0 del; 19 mod 8263892: More modifier order fixes in java.base Reviewed-by: naoto, iris, redestad - PR: https://git.openjdk.java.net/jdk/pull/3094
Re: RFR: 8263892: More modifier order fixes in java.base
On Fri, 19 Mar 2021 18:23:00 GMT, Alex Blewitt wrote: > Additional changes found in `java.base` of `final private` -> `private > final`. Filed with existing bug because it's the same module; can change to a > different bug number if required. Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3094
Integrated: 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa
On Fri, 19 Mar 2021 15:27:00 GMT, Alex Blewitt wrote: > Fixes for the `java.sql`, `java.sql.rowset` and `java.transaction.xa` > packages. > > @cl4es would you mind creating a subtask of JDK-8263854 so I can update the > PR title? This pull request has now been integrated. Changeset: 80d3ea02 Author:Alex Blewitt Committer: Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/80d3ea02 Stats: 133 lines in 6 files changed: 0 ins; 0 del; 133 mod 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa Reviewed-by: redestad, lancea - PR: https://git.openjdk.java.net/jdk/pull/3090
Re: RFR: 8263892: More modifier order fixes in java.base
On Fri, 19 Mar 2021 18:23:00 GMT, Alex Blewitt wrote: > Additional changes found in `java.base` of `final private` -> `private > final`. Filed with existing bug because it's the same module; can change to a > different bug number if required. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3094
Integrated: JDK-8263545: Convert jpackage to use Stream.toList()
On Sun, 14 Mar 2021 18:22:50 GMT, Ian Graves wrote: > This converts jpackage to use `Stream.toList()` instead of > `Stream.collect(Collectors.toList())`. One piece of code was modified to not > mutate a list in addition to one test that used a mutating sort on a list. > The rest of the changes are simple substitutions. This pull request has now been integrated. Changeset: 0b5216a9 Author:Ian Graves Committer: Alexey Semenyuk URL: https://git.openjdk.java.net/jdk/commit/0b5216a9 Stats: 47 lines in 17 files changed: 1 ins; 3 del; 43 mod 8263545: Convert jpackage to use Stream.toList() Reviewed-by: asemenyuk, almatvee - PR: https://git.openjdk.java.net/jdk/pull/2997
Re: RFR: 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa
On Fri, 19 Mar 2021 15:27:00 GMT, Alex Blewitt wrote: > Fixes for the `java.sql`, `java.sql.rowset` and `java.transaction.xa` > packages. > > @cl4es would you mind creating a subtask of JDK-8263854 so I can update the > PR title? Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3090
Re: RFR: 8263890: Broken links to Unicode.org
On Fri, 19 Mar 2021 17:57:31 GMT, Naoto Sato wrote: > Fixed several broken links to Unicode.org. Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 8263890: Broken links to Unicode.org
On Fri, 19 Mar 2021 17:57:31 GMT, Naoto Sato wrote: > Fixed several broken links to Unicode.org. Some minor comments. src/java.base/share/classes/java/text/Collator.java line 211: > 209: * FULL_DECOMPOSITION corresponds to Normalization Form KD as > 210: * described in > 211: * http://www.unicode.org/reports/tr15/tr15-23.html;>Unicode While the previous "Unicode Technical Report #15" was in http://unicode.org/reports/tr15/tr15-15.html, by tr15-23 it was "approved by the Unicode Technical Committee as a Unicode Standard Annex". Do we want to change the title to "Unicode Standard Annex #15"? Also, is Collator stuck with a particular revision (e.g. 23) or do we refer to the latest, e.g. "http://www.unicode.org/reports/tr15/;? src/java.base/share/classes/jdk/internal/icu/text/BidiLine.java line 50: > 48: * which has already been processed according to > 49: * the Unicode 3.0 Bidi algorithm as defined in > 50: * http://www.unicode.org/reports/tr9/ , version 13, Previous references provided links e.g. "http://www.unicode.org/unicode/reports/tr9/;>Unicode Standard Annex #9" instead of URL. src/java.base/share/classes/jdk/internal/icu/text/Normalizer2.java line 46: > 44: * a string is already normalized. > 45: * The most commonly used normalization forms are those defined in > 46: * http://www.unicode.org/reports/tr15/ Same as above. src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xpath/regex/RegularExpression.java line 476: > 474: * TODO > 475: * > 476: * http://www.unicode.org/reports/tr18/;>Unicode Regular > Expression Guidelines Just a note that the references had been report numbers, e.g. Report/Annex #15, while this one the Title (by the way, the official title was "Unicode Regular Expressions"). The only exception seems to be the one in NormalizerBase.java where it referred to the number + title. - Marked as reviewed by joehw (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v4]
> Intermittent failures on Windows in a test of destroying the child warrant > extending the time the parent waits after starting the child before > destroying the child. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Clarify modification of argument list after creating ProcessBuilder - Changes: - all: https://git.openjdk.java.net/jdk/pull/3049/files - new: https://git.openjdk.java.net/jdk/pull/3049/files/6313399c..31c35beb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3049=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3049=02-03 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3049/head:pull/3049 PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Fri, 19 Mar 2021 18:31:02 GMT, Naoto Sato wrote: >> Additional changes found in `java.base` of `final private` -> `private >> final`. Filed with existing bug because it's the same module; can change to >> a different bug number if required. > > Marked as reviewed by naoto (Reviewer). Can't reuse the bug IDs, sadly. Filed https://bugs.openjdk.java.net/browse/JDK-8263892 (note the new summary name to disambiguate) - PR: https://git.openjdk.java.net/jdk/pull/3094
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Fri, 19 Mar 2021 18:23:00 GMT, Alex Blewitt wrote: > Additional changes found in `java.base` of `final private` -> `private > final`. Filed with existing bug because it's the same module; can change to a > different bug number if required. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3094
RFR: 8263658: Use the blessed modifier order in java.base
Additional changes found in `java.base` of `final private` -> `private final`. Filed with existing bug because it's the same module; can change to a different bug number if required. - Commit messages: - 8263658: Use the blessed modifier order in java.base Changes: https://git.openjdk.java.net/jdk/pull/3094/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3094=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263658 Stats: 19 lines in 5 files changed: 0 ins; 0 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/3094.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3094/head:pull/3094 PR: https://git.openjdk.java.net/jdk/pull/3094
Re: RFR: 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa
On Fri, 19 Mar 2021 15:27:00 GMT, Alex Blewitt wrote: > Fixes for the `java.sql`, `java.sql.rowset` and `java.transaction.xa` > packages. > > @cl4es would you mind creating a subtask of JDK-8263854 so I can update the > PR title? Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3090
RFR: 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa
Fixes for the `java.sql`, `java.sql.rowset` and `java.transaction.xa` packages. @cl4es would you mind creating a subtask of JDK-8263854 so I can update the PR title? - Commit messages: - Use the blessed modifier order in java.sql/rowset/transation.xa Changes: https://git.openjdk.java.net/jdk/pull/3090/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3090=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263885 Stats: 133 lines in 6 files changed: 0 ins; 0 del; 133 mod Patch: https://git.openjdk.java.net/jdk/pull/3090.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3090/head:pull/3090 PR: https://git.openjdk.java.net/jdk/pull/3090
Re: RFR: 8263885: Use the blessed modifier order in java.sql/rowset/transation.xa
On Fri, 19 Mar 2021 15:27:00 GMT, Alex Blewitt wrote: > Fixes for the `java.sql`, `java.sql.rowset` and `java.transaction.xa` > packages. > > @cl4es would you mind creating a subtask of JDK-8263854 so I can update the > PR title? 8263885 - PR: https://git.openjdk.java.net/jdk/pull/3090
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v3]
On Fri, 19 Mar 2021 17:17:02 GMT, Roger Riggs wrote:
>> Intermittent failures on Windows in a test of destroying the child warrant
>> extending the time the parent waits after starting the child before
>> destroying the child.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Correct redirect of stdout
LGTM. Just a small nit.
test/jdk/java/lang/ProcessBuilder/Basic.java line 2140:
> 2138: List childArgs = new ArrayList<>(javaChildArgs);
> 2139: final ProcessBuilder pb = new ProcessBuilder(childArgs);
> 2140: {
I was a little confused until I looked up the JavaDoc of ProcessBuilder.
Maybe add a comment like "ProcessBuilder does NOT make a copy of childArgs so
it's OK to modify it below"?
-
Marked as reviewed by iklam (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263890: Broken links to Unicode.org
On Fri, 19 Mar 2021 17:57:31 GMT, Naoto Sato wrote: > Fixed several broken links to Unicode.org. Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3093
RFR: 8263890: Broken links to Unicode.org
Fixed several broken links to Unicode.org. - Commit messages: - 8263890: Broken links to Unicode.org Changes: https://git.openjdk.java.net/jdk/pull/3093/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3093=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263890 Stats: 22 lines in 8 files changed: 0 ins; 0 del; 22 mod Patch: https://git.openjdk.java.net/jdk/pull/3093.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3093/head:pull/3093 PR: https://git.openjdk.java.net/jdk/pull/3093
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming [v2]
On Fri, 19 Mar 2021 17:13:58 GMT, Alex Blewitt wrote: >> As with #2993 changing the order of `final static` to `static final` for the >> `java.management`, `java.management.rmi` and `java.naming` modules. > > Alex Blewitt has updated the pull request incrementally with one additional > commit since the last revision: > > Added more replacements of 'final public' with 'public final' Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]
On Fri, 19 Mar 2021 17:01:26 GMT, Roger Riggs wrote: >> test/jdk/java/lang/ProcessBuilder/Basic.java line 2156: >> >>> 2154: case 1: >>> 2155: childArgs.set(1, >>> "-XX:+DisplayVMOutputToStdout"); >>> 2156: pb.redirectInput(INHERIT); >> >> not `redirectOutput()` ? > > You are correct. will fix. > (The Process streams are from the point of view of parent). Yes, I have to look this up every time. Its not intuitive. - PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: JDK-8263545: Convert jpackage to use Stream.toList() [v3]
On Fri, 19 Mar 2021 01:17:34 GMT, Alexey Semenyuk wrote: >> Ian Graves has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains two commits: >> >> - Clarifying type argument and shorting a toArray invocation >> - Converting jpackage to use Stream.toList() > > Marked as reviewed by asemenyuk (Reviewer). Ready for sponsoring when you all are. - PR: https://git.openjdk.java.net/jdk/pull/2997
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v3]
> Intermittent failures on Windows in a test of destroying the child warrant > extending the time the parent waits after starting the child before > destroying the child. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Correct redirect of stdout - Changes: - all: https://git.openjdk.java.net/jdk/pull/3049/files - new: https://git.openjdk.java.net/jdk/pull/3049/files/d8d4fccc..6313399c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3049=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3049=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3049/head:pull/3049 PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming [v2]
> As with #2993 changing the order of `final static` to `static final` for the > `java.management`, `java.management.rmi` and `java.naming` modules. Alex Blewitt has updated the pull request incrementally with one additional commit since the last revision: Added more replacements of 'final public' with 'public final' - Changes: - all: https://git.openjdk.java.net/jdk/pull/3078/files - new: https://git.openjdk.java.net/jdk/pull/3078/files/f1bc7269..1c900d2d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3078=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3078=00-01 Stats: 106 lines in 37 files changed: 0 ins; 0 del; 106 mod Patch: https://git.openjdk.java.net/jdk/pull/3078.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3078/head:pull/3078 PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]
On Fri, 19 Mar 2021 16:54:43 GMT, Thomas Stuefe wrote: >> Roger Riggs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Redirect VM output away from stream under test > > test/jdk/java/lang/ProcessBuilder/Basic.java line 2156: > >> 2154: case 1: >> 2155: childArgs.set(1, >> "-XX:+DisplayVMOutputToStdout"); >> 2156: pb.redirectInput(INHERIT); > > not `redirectOutput()` ? The naming of the methods is from the point of view of the parent, not the child. So it seems backward, but is correct. - PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]
On Fri, 19 Mar 2021 14:23:55 GMT, Roger Riggs wrote: >> Intermittent failures on Windows in a test of destroying the child warrant >> extending the time the parent waits after starting the child before >> destroying the child. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Redirect VM output away from stream under test Hi Roger, thanks for taking my suggestion. Minor point below. Cheers, Thomas test/jdk/java/lang/ProcessBuilder/Basic.java line 2156: > 2154: case 1: > 2155: childArgs.set(1, > "-XX:+DisplayVMOutputToStdout"); > 2156: pb.redirectInput(INHERIT); not `redirectOutput()` ? - PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices [v2]
> 8263763: The constructor of an enumeration prefixes with two synthetic > arguments for constant name and ordinal index. For this reason, the > constructor annotations need to be shifted two indices to the right, such > that the annotation indices match with the parameter indices. Rafael Winterhalter has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8263763: The constructor of an enumeration prefixes with two synthetic arguments for constant name and ordinal index. For this reason, the constructor annotations need to be shifted two indices to the right, such that the annotation indices match with the parameter indices. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3082/files - new: https://git.openjdk.java.net/jdk/pull/3082/files/1170d79d..d50d5f4c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3082=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3082=00-01 Stats: 8 lines in 3 files changed: 3 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/3082.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3082/head:pull/3082 PR: https://git.openjdk.java.net/jdk/pull/3082
Re: RFR: 8263763: Synthetic constructor parameters of enum are not considered for annotation indices
On Fri, 19 Mar 2021 05:24:24 GMT, Joe Darcy wrote: >> 8263763: The constructor of an enumeration prefixes with two synthetic >> arguments for constant name and ordinal index. For this reason, the >> constructor annotations need to be shifted two indices to the right, such >> that the annotation indices match with the parameter indices. > > A general comment, the enum constructor situation is a bit tricky as > > 1) There is no 100% reliable way to determine which of a enum constructor's > args are synthetic. > > 2) How a Java compiler generates enum constructors is a compiler-internal > contract since all the enum constructors must be private. > > It is true that javac has consistently added two extra parameters as an > implementation detail. Offhand, I don't know if ecj in particular has made > the same choice. javac is not obliged to continue implementing enum > constructors in this manner, so for better robustness, I think a fix in this > space needs to be able to accommodate situations other than exactly N+2 > parameters. *ejc* issues the same constructor that prefixes a `String` and an `int` parameter. I agree that the solution is not perfect, but I would prefer it over the reflection API throwing an `IndexOutOfBoundsException` upon calling `getAnnotations()`, which nobody really expects upon a getter invocation. I added a check to see if the enum constructor in question starts with a `String` and `int` parameter after checking if there are two excess parameters. I believe that that's at least an improvement over today's state where it's very unlikely that an enum constructor yields an incorrect result in the reflection API whereas the result it is always incorrect today, given the implementation of both *ejc* and *javac*. Ideally, this would of course also be fixed by javac (and ejc) such that the annotations are placed on the correct indices, but I still argue that the fix is an improvement for being able to properly process enum constructors for older class files also. - PR: https://git.openjdk.java.net/jdk/pull/3082
Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v2]
> Intermittent failures on Windows in a test of destroying the child warrant > extending the time the parent waits after starting the child before > destroying the child. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Redirect VM output away from stream under test - Changes: - all: https://git.openjdk.java.net/jdk/pull/3049/files - new: https://git.openjdk.java.net/jdk/pull/3049/files/ba3d9479..d8d4fccc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3049=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3049=00-01 Stats: 28 lines in 1 file changed: 23 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/3049.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3049/head:pull/3049 PR: https://git.openjdk.java.net/jdk/pull/3049
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Thu, 18 Mar 2021 18:26:20 GMT, Alex Blewitt
wrote:
> As with #2993 changing the order of `final static` to `static final` for the
> `java.management`, `java.management.rmi` and `java.naming` modules.
Overall good - found a few cases where the private modifier is in the wrong
place that ought to be fixed as well.
src/java.naming/share/classes/com/sun/jndi/ldap/EventQueue.java line 48:
> 46: */
> 47: final class EventQueue implements Runnable {
> 48: static final private boolean debug = false;
Move private to the front?
src/java.naming/share/classes/com/sun/jndi/ldap/EventSupport.java line 116:
> 114: */
> 115: final class EventSupport {
> 116: static final private boolean debug = false;
Move private to the front?
src/java.naming/share/classes/com/sun/jndi/ldap/LdapSchemaCtx.java line 389:
> 387: }
> 388:
> 389: static final private class SchemaInfo {
Move private to the front?
-
Changes requested by redestad (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Fri, 19 Mar 2021 13:28:12 GMT, Claes Redestad wrote: >> Thanks @cl4es -- do I need to update the git commit message as well, or is >> updating the title of the PR sufficient? I recall you suggesting not to do >> amend/rebases previously. > > No, the git commit messages here doesn't matter, the bots will use the PR > name when merging into master. It's good form to use reasonably consistent > commit messages as you add commits to a PR, but altering commit history is > not necessary and might even be disruptive once a PR has been opened. > > There's the /summary command you could use to add additional comments to the > final commit, see > https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/summary Thanks @cl4es will adjust my submissions appropriately in the future. - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Fri, 19 Mar 2021 13:08:24 GMT, Alex Blewitt wrote: >> Created the subtask https://bugs.openjdk.java.net/browse/JDK-8263855 for >> this along with an umbrella RFE. > > Thanks @cl4es -- do I need to update the git commit message as well, or is > updating the title of the PR sufficient? I recall you suggesting not to do > amend/rebases previously. No, the git commit messages here doesn't matter, the bots will use the PR name when merging into master. It's good form to use reasonably consistent commit messages as you add commits to a PR, but altering commit history is not necessary and might even be disruptive once a PR has been opened. There's the /summary command you could use to add additional comments to the final commit, see https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/summary - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Fri, 19 Mar 2021 10:20:39 GMT, Alex Blewitt wrote: >> Would someone mind creating a bug for me? In addition, if it would help, we >> could create a parent bug for applying cleanups and associate #2993 and >> #2991. > > @cl4es would you mind creating a parent task of something like "Source code > cleanups" and then another sub task for this change please? Created the subtask https://bugs.openjdk.java.net/browse/JDK-8263855 for this along with an umbrella RFE. - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Fri, 19 Mar 2021 11:01:45 GMT, Claes Redestad wrote: >> @cl4es would you mind creating a parent task of something like "Source code >> cleanups" and then another sub task for this change please? > > Created the subtask https://bugs.openjdk.java.net/browse/JDK-8263855 for this > along with an umbrella RFE. Thanks @cl4es -- do I need to update the git commit message as well, or is updating the title of the PR sufficient? I recall you suggesting not to do amend/rebases previously. - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Thu, 18 Mar 2021 19:33:59 GMT, Alex Blewitt wrote: >> As with #2993 changing the order of `final static` to `static final` for the >> `java.management`, `java.management.rmi` and `java.naming` modules. > > Would someone mind creating a bug for me? In addition, if it would help, we > could create a parent bug for applying cleanups and associate #2993 and > #2991. @cl4es would you mind creating a parent task of something like "Source code cleanups" and then another sub task for this change please? - PR: https://git.openjdk.java.net/jdk/pull/3078
Re: RFR: 8263855: Use the blessed modifier order in java.management/naming
On Thu, 18 Mar 2021 18:26:20 GMT, Alex Blewitt wrote: > As with #2993 changing the order of `final static` to `static final` for the > `java.management`, `java.management.rmi` and `java.naming` modules. Would someone mind creating a bug for me? In addition, if it would help, we could create a parent bug for applying cleanups and associate #2993 and #2991. - PR: https://git.openjdk.java.net/jdk/pull/3078
RFR: 8263855: Use the blessed modifier order in java.management/naming
As with #2993 changing the order of `final static` to `static final` for the `java.management`, `java.management.rmi` and `java.naming` modules. - Commit messages: - Use the blessed modifier order in java.management/naming Changes: https://git.openjdk.java.net/jdk/pull/3078/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3078=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263855 Stats: 158 lines in 39 files changed: 0 ins; 0 del; 158 mod Patch: https://git.openjdk.java.net/jdk/pull/3078.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3078/head:pull/3078 PR: https://git.openjdk.java.net/jdk/pull/3078
Integrated: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 This pull request has now been integrated. Changeset: b49c5893 Author:Alex Blewitt Committer: Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/b49c5893 Stats: 139 lines in 28 files changed: 0 ins; 0 del; 139 mod 8263658: Use the blessed modifier order in java.base Reviewed-by: rriggs, redestad - PR: https://git.openjdk.java.net/jdk/pull/2993
Integrated: 8263821: Remove unused MethodTypeForm canonicalization codes
On Thu, 18 Mar 2021 15:42:49 GMT, Claes Redestad wrote: > MethodTypeForm.INTS, LONGS and RAW_RETURN are effectively unused. This > removes these canonicalization codes and cleans up related code. This pull request has now been integrated. Changeset: 57497ab0 Author:Claes Redestad URL: https://git.openjdk.java.net/jdk/commit/57497ab0 Stats: 45 lines in 2 files changed: 0 ins; 35 del; 10 mod 8263821: Remove unused MethodTypeForm canonicalization codes Reviewed-by: mchung - PR: https://git.openjdk.java.net/jdk/pull/3075
Re: RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields [v3]
> 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional > GZIP fields Lin Zang has updated the pull request incrementally with two additional commits since the last revision: - update copyright - reuse arguments constructor for non-argument one. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3072/files - new: https://git.openjdk.java.net/jdk/pull/3072/files/533199ef..9b7dabfe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3072=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3072=01-02 Stats: 14 lines in 1 file changed: 0 ins; 11 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/3072.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3072/head:pull/3072 PR: https://git.openjdk.java.net/jdk/pull/3072
Re: RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields [v2]
> 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional > GZIP fields Lin Zang has updated the pull request incrementally with one additional commit since the last revision: add test case - Changes: - all: https://git.openjdk.java.net/jdk/pull/3072/files - new: https://git.openjdk.java.net/jdk/pull/3072/files/8cdc1fa6..533199ef Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3072=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3072=00-01 Stats: 62 lines in 1 file changed: 58 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/3072.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3072/head:pull/3072 PR: https://git.openjdk.java.net/jdk/pull/3072
Re: RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields
On Thu, 18 Mar 2021 10:50:05 GMT, Lin Zang wrote: >> 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional >> GZIP fields > > Dear All, > This PR introduce new constructor of GZIPOutputStream to support adding extra > field info in gzip file header, as decribed in RFC 1952 gzip spec > (https://tools.ietf.org/html/rfc1952). > BTW, does a CSR required for this PR? > > Thanks! > Lin The CSR has been submitted at https://bugs.openjdk.java.net/browse/JDK-8263793 - PR: https://git.openjdk.java.net/jdk/pull/3072
