Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8253497: Core Libs Terminology Refresh #1771

Closed
wants to merge 8 commits into from

Conversation

@bchristi-git
Copy link
Member

@bchristi-git bchristi-git commented Dec 14, 2020

This is part of an effort in the JDK to replace archaic/non-inclusive words with more neutral terms (see JDK-8253315 for details).

Here are the changes covering core libraries code and tests. Terms were changed as follows:

  1. grandfathered -> legacy
  2. blacklist -> filter or reject
  3. whitelist -> allow or accept
  4. master -> coordinator
  5. slave -> worker

Addressing similar issues in upstream 3rd party code is out of scope of this PR. Such changes will be picked up from their upstream sources.


Progress

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

Issue

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1771/head:pull/1771
$ git checkout pull/1771

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Dec 14, 2020

👋 Welcome back bchristi! 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 openjdk bot added the rfr label Dec 14, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Dec 14, 2020

@bchristi-git The following labels will be automatically applied to this pull request:

  • build
  • core-libs
  • i18n
  • jmx
  • nio
  • serviceability

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
Copy link

@mlbridge mlbridge bot commented Dec 14, 2020

naotoj
naotoj approved these changes Dec 14, 2020
Copy link
Member

@naotoj naotoj left a comment

Looks good to me.

@@ -62,7 +62,7 @@ public static void main(String[] args) throws Exception {
int[] switches = new int[7];

int switchSource = 0;
if (args.length == 0) { // This is master version
if (args.length == 0) { // This is the coordinator version
Copy link
Member

@naotoj naotoj Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space between "is" and "the."

@openjdk
Copy link

@openjdk openjdk bot commented Dec 14, 2020

@bchristi-git 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:

8253497: Core Libs Terminology Refresh

Reviewed-by: naoto, kcr, rriggs, joehw, bpb, smarks, alanb

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 41 new commits pushed to the master branch:

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.

@openjdk openjdk bot added the ready label Dec 14, 2020
@@ -132,7 +132,7 @@
* the serial form of any deserialized object.
* The pattern must be in same format as used in
* {@link java.io.ObjectInputFilter.Config#createFilter}.
* It may define a white list of permitted classes, a black list of
* It may define an accept-list of permitted classes, a reject-list of
Copy link
Contributor

@RogerRiggs RogerRiggs Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept -> allow for consistency

@@ -149,7 +149,7 @@
* classes they use in their serial form.
* <p>
* Care must be taken when defining such a filter, as defining
* a white list too restrictive or a too wide a black list may
* an accept-list too restrictive or a too-wide reject-list may
Copy link
Contributor

@RogerRiggs RogerRiggs Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept -> allow for consistency

@@ -132,7 +132,7 @@
* the serial form of any deserialized object.
* The pattern must be in same format as used in
* {@link java.io.ObjectInputFilter.Config#createFilter}.
* It may define a white list of permitted classes, a black list of
* It may define an accept-list of permitted classes, a reject-list of
Copy link
Member

@JoeWang-Java JoeWang-Java Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should accept-list be allow-list to be consistent with the other two RMI classes and ObjectInputFilter.Status#ALLOWED?

@@ -149,7 +149,7 @@
* classes they use in their serial form.
* <p>
* Care must be taken when defining such a filter, as defining
* a white list too restrictive or a too wide a black list may
* an accept-list too restrictive or a too-wide reject-list may
Copy link
Member

@JoeWang-Java JoeWang-Java Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would "an allow-list too restrictive or a reject-list too wide" read better?

Copy link
Member Author

@bchristi-git bchristi-git Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that there is room for improvement here. How about:
"...an allow-list too restrictively, or a reject-list too broadly, may..."
?

Copy link
Member

@JoeWang-Java JoeWang-Java Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your call, I'm not a native English speaker :-) It felt to me it's 'restrictive' than 'restrictively', an adj placed after the noun, e.g. a restrictive allow-list.

Copy link
Member

@magicus magicus Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an adverb, since it's the act of 'defining' that is being done too restrictively or broadly. If you want to have an adjective you would need to rephrase it so it applies to the noun, like 'defining a too restrictive accept-list'. My non-native English vibes would still prefer the former.

Copy link
Member

@stuart-marks stuart-marks Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest ... as defining an allow-list that is too narrow or a reject-list that is too wide may prevent ....

(edited to remove hyphen from "too-wide")

Copy link
Member Author

@bchristi-git bchristi-git Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even better!

@@ -42,8 +42,8 @@ public static void main(String[] args) throws Exception {

SocketChannel client = SocketChannel.open ();
client.connect (new InetSocketAddress (InetAddress.getLoopbackAddress(), port));
SocketChannel slave = server.accept ();
slave.configureBlocking (true);
SocketChannel channel = server.accept ();
Copy link
Contributor

@AlanBateman AlanBateman Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to "peer" instead? (we can remove the spurious space after accept while we are there).

Copy link
Member Author

@bchristi-git bchristi-git Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will name it, "peer", and remove the extra space from the affected lines.

@magicus
Copy link
Member

@magicus magicus commented Dec 15, 2020

/label remove build

@openjdk openjdk bot removed the build label Dec 15, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Dec 15, 2020

@magicus
The build label was successfully removed.

@@ -62,7 +62,7 @@ public static void main(String[] args) throws Exception {
int[] switches = new int[7];

int switchSource = 0;
if (args.length == 0) { // This is master version
if (args.length == 0) { // This is the coordinator version
Copy link
Member

@bplb bplb Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps s/coordinator/controller/?

Copy link
Contributor

@LanceAndersen LanceAndersen Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change it to:

// This is the controller

Copy link
Member Author

@bchristi-git bchristi-git Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

bplb
bplb approved these changes Dec 15, 2020
@@ -1645,18 +1645,19 @@ public String toLanguageTag() {
* </pre></ul>
*
* <p>This implements the 'Language-Tag' production of BCP47, and
* so supports grandfathered (regular and irregular) as well as
* so supports legacy (regular and irregular, referred to as
* {@code Type: grandfathered} in BCP47) as well as
Copy link
Contributor

@AlanBateman AlanBateman Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider putting quotes around "Type; grandfathered" here, otherwise looks good.

Copy link
Member Author

@bchristi-git bchristi-git Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, having that in quotes instead of in {@code} would be consistent with the rest of Locale.java. I will change that.

@bchristi-git
Copy link
Member Author

@bchristi-git bchristi-git commented Dec 16, 2020

/integrate

@openjdk openjdk bot closed this Dec 16, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Dec 16, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Dec 16, 2020

@bchristi-git Since your change was applied there have been 41 commits pushed to the master branch:

Your commit was automatically rebased without conflicts.

Pushed as commit b2f0355.

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

@bchristi-git bchristi-git deleted the 8253497 branch May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment