8269513: Clarify the spec wrt `useOldISOCodes` system property #163

naotoj · 2021-06-28T16:57:15Z

Please review this small doc change to the system property. Accompanying CSR has also been created.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8269513: Clarify the spec wrt useOldISOCodes system property

Reviewers

Lance Andersen (@LanceAndersen - Reviewer)
Brian Burkhalter (@bplb - Reviewer) ⚠️ Review applies to 3d25418
Iris Clark (@irisclark - Reviewer)
Joe Wang (@JoeWang-Java - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk17 pull/163/head:pull/163
$ git checkout pull/163

Update a local copy of the PR:
$ git checkout pull/163
$ git pull https://git.openjdk.java.net/jdk17 pull/163/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 163

View PR using the GUI difftool:
$ git pr show -t 163

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk17/pull/163.diff

naotoj · 2021-06-28T16:57:24Z

/csr

bridgekeeper · 2021-06-28T16:57:56Z

👋 Welcome back naoto! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-06-28T16:59:32Z

@naotoj this pull request will not be integrated until the CSR request JDK-8269531 for issue JDK-8269513 has been approved.

openjdk · 2021-06-28T17:01:26Z

@naotoj The following labels will be automatically applied to this pull request:

core-libs
i18n

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2021-06-28T17:06:17Z

Webrevs

LanceAndersen

Looks good Naoto

bplb

Looks fine.

LanceAndersen · 2021-06-28T18:45:31Z

src/java.base/share/classes/java/util/Locale.java

- * so the later call to {@code System.setProperty()} won't affect the backward
+ * back to that of before Java SE 17. If the system property is set to
+ * {@code true}, those three current language codes are mapped to their
+ * backward compatible forms. It is only read at Java runtime startup, so a


I had thought about some of some minor word smithing in your prior commit, but chose not to request a change.

In the above sentence, It could be clearer what "It" is. Perhaps something along the lines of:

This property is only read at Java runtime startup and subsequents calls to ...."

Also, "backward compatible behavior" maybe not need to be repeated, e.g. "subsequent calls to ... will have no effect.

"back to that of before Java SE 17" -- "of" may be removed.

JoeWang-Java · 2021-06-28T19:23:28Z

src/java.base/share/classes/java/util/Locale.java

- * so the later call to {@code System.setProperty()} won't affect the backward
+ * back to that of before Java SE 17. If the system property is set to
+ * {@code true}, those three current language codes are mapped to their
+ * backward compatible forms. It is only read at Java runtime startup, so a


Also, "backward compatible behavior" maybe not need to be repeated, e.g. "subsequent calls to ... will have no effect.

"back to that of before Java SE 17" -- "of" may be removed.

naotoj · 2021-06-28T20:26:16Z

Thanks. I updated the PR and CSR accordingly. As to removing "of", I think the current form is fine, but I am not a native English speaker, so happy to correct it if it is wrong.

JoeWang-Java · 2021-06-28T22:23:43Z

Thanks for the update. As for "of", apparently the native speakers were fine with it ;-)

openjdk · 2021-06-30T22:08:32Z

@naotoj This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8269513: Clarify the spec wrt `useOldISOCodes` system property

Reviewed-by: lancea, bpb, iris, joehw

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 24 new commits pushed to the master branch:

0dc65d3: 8268897: [TESTBUG] compiler/compilercontrol/mixed/RandomCommandsTest.java must not fail on Command.quiet
3826012: 8268557: Module page uses unstyled table class
2b17e95: 8269691: ProblemList sun/management/jdp/JdpDefaultsTest.java on Linux-aarch64
1da5d4b: 8269486: CallerAccessTest fails for non server variant
be0ac92: 8269614: [s390] Interpreter checks wrong bit for slow path instance allocation
4b4bef4: 8269594: assert(_handle_mark_nesting > 1) failed: memory leak: allocating handle outside HandleMark
4ee400a: 8268320: Better error recovery for broken patterns in switch
ca283c3: 8265907: JVM crashes when matching VectorMaskCmp Node
c3c9189: 8269141: Switch statement containing pattern case label element gets in the loop during execution
6b64a79: 8268350: Remove assert that ensures thread identifier remains the same
... and 14 more: https://git.openjdk.java.net/jdk17/compare/8d09596f270c822b867d38a5462ec62c613f1d0e...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

naotoj · 2021-06-30T22:10:50Z

/integrate

openjdk · 2021-06-30T22:12:06Z

Going to push as commit 3e02224.
Since your change was applied there have been 24 commits pushed to the master branch:

0dc65d3: 8268897: [TESTBUG] compiler/compilercontrol/mixed/RandomCommandsTest.java must not fail on Command.quiet
3826012: 8268557: Module page uses unstyled table class
2b17e95: 8269691: ProblemList sun/management/jdp/JdpDefaultsTest.java on Linux-aarch64
1da5d4b: 8269486: CallerAccessTest fails for non server variant
be0ac92: 8269614: [s390] Interpreter checks wrong bit for slow path instance allocation
4b4bef4: 8269594: assert(_handle_mark_nesting > 1) failed: memory leak: allocating handle outside HandleMark
4ee400a: 8268320: Better error recovery for broken patterns in switch
ca283c3: 8265907: JVM crashes when matching VectorMaskCmp Node
c3c9189: 8269141: Switch statement containing pattern case label element gets in the loop during execution
6b64a79: 8268350: Remove assert that ensures thread identifier remains the same
... and 14 more: https://git.openjdk.java.net/jdk17/compare/8d09596f270c822b867d38a5462ec62c613f1d0e...master

Your commit was automatically rebased without conflicts.

openjdk · 2021-06-30T22:12:22Z

@naotoj Pushed as commit 3e02224.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8269513: Clarify the spec wrt useOldISOCodes system property

92a8bfc

openjdk bot added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Jun 28, 2021

openjdk bot added core-libs core-libs-dev@openjdk.java.net i18n i18n-dev@openjdk.java.net labels Jun 28, 2021

LanceAndersen approved these changes Jun 28, 2021

View reviewed changes

Refined wording.

3d25418

bplb approved these changes Jun 28, 2021

View reviewed changes

LanceAndersen reviewed Jun 28, 2021

View reviewed changes

irisclark approved these changes Jun 28, 2021

View reviewed changes

JoeWang-Java approved these changes Jun 28, 2021

View reviewed changes

Refined wording #2.

da11921

Missed one.

3b6ff65

irisclark approved these changes Jun 28, 2021

View reviewed changes

LanceAndersen approved these changes Jun 28, 2021

View reviewed changes

JoeWang-Java approved these changes Jun 28, 2021

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label Jun 30, 2021

openjdk bot added the ready Pull request is ready to be integrated label Jun 30, 2021

openjdk bot closed this Jun 30, 2021

openjdk bot added the integrated Pull request has been integrated label Jun 30, 2021

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 30, 2021

8269513: Clarify the spec wrt useOldISOCodes system property #163

8269513: Clarify the spec wrt useOldISOCodes system property #163

Uh oh!

Conversation

naotoj commented Jun 28, 2021 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

naotoj commented Jun 28, 2021

Uh oh!

bridgekeeper bot commented Jun 28, 2021

Uh oh!

openjdk bot commented Jun 28, 2021

Uh oh!

openjdk bot commented Jun 28, 2021

Uh oh!

mlbridge bot commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

LanceAndersen left a comment

Choose a reason for hiding this comment

Uh oh!

bplb left a comment

Choose a reason for hiding this comment

Uh oh!

LanceAndersen Jun 28, 2021

Choose a reason for hiding this comment

Uh oh!

JoeWang-Java Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoeWang-Java Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naotoj commented Jun 28, 2021

Uh oh!

JoeWang-Java commented Jun 28, 2021

Uh oh!

openjdk bot commented Jun 30, 2021

Uh oh!

naotoj commented Jun 30, 2021

Uh oh!

openjdk bot commented Jun 30, 2021

Uh oh!

openjdk bot commented Jun 30, 2021

Uh oh!

Uh oh!

8269513: Clarify the spec wrt `useOldISOCodes` system property #163

8269513: Clarify the spec wrt `useOldISOCodes` system property #163

naotoj commented Jun 28, 2021 •

edited by openjdk bot

Loading

mlbridge bot commented Jun 28, 2021 •

edited

Loading

JoeWang-Java Jun 28, 2021 •

edited

Loading

JoeWang-Java Jun 28, 2021 •

edited

Loading