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

Deprecating master terminology to support inclusive naming #167

Merged
merged 2 commits into from
Jun 27, 2022

Conversation

VachaShah
Copy link
Collaborator

Signed-off-by: Vacha Shah [email protected]

Description

Deprecating master nomenclature for inclusive naming which includes the following changes:

  • master -> cluster-manager in comments
  • master -> clusterManager in methods and variables
  • master -> cluster_manager in API parameters

Issues Resolved

#166

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@VachaShah VachaShah requested review from a team and madhusudhankonda as code owners June 16, 2022 01:40
@VachaShah VachaShah requested a review from tlfeng June 16, 2022 01:40
@@ -140,9 +140,13 @@ public class NodesRecord implements JsonpSerializable {
@Nullable
private final String nodeRole;

@Deprecated
Copy link

Choose a reason for hiding this comment

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

nit: I think technically private variable or method can be renamed freely without deprecating. 😁

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Missed this, I will update it 🤦🏼‍♀️

Copy link

@tlfeng tlfeng Jun 16, 2022

Choose a reason for hiding this comment

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

I think that is fine if not changed, while reducing the code change maybe better. 😄

*/
public static final MasterRequest _INSTANCE = new MasterRequest();
public static final ClusterManagerRequest _INSTANCE = new ClusterManagerRequest();

// ---------------------------------------------------------------------------------------------

/**
* Endpoint "{@code cat.master}".
Copy link
Member

Choose a reason for hiding this comment

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

If we have changed the API request in this PR
master -> cluster_manager

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean for the comment cat.master? cat.master is for the deprecated endpoint, cat.cluster_manager is for the endpoint.

Copy link
Member

Choose a reason for hiding this comment

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

I see we are supporting the deprecated and the new endpoint to not break things out there. Makes sense. You can ignore my comment.

@VachaShah VachaShah requested review from tlfeng and owaiskazi19 June 16, 2022 19:53
Copy link

@tlfeng tlfeng left a comment

Choose a reason for hiding this comment

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

It's really impressive to make those changes to support inclusive language. 👍 👍

@VachaShah VachaShah requested a review from CEHENKLE June 23, 2022 22:39
@VachaShah VachaShah merged commit fe7b414 into opensearch-project:main Jun 27, 2022
@VachaShah VachaShah deleted the inclusive-naming branch June 27, 2022 17:41
mtimmerm pushed a commit to mtimmerm/opensearch-java that referenced this pull request Jul 1, 2022
…h-project#167)

* Deprecating master terminology to support inclusive naming

Signed-off-by: Vacha Shah <[email protected]>

* Fixing tests against unreleased OpenSearch

Signed-off-by: Vacha Shah <[email protected]>
Signed-off-by: Matt Timmermans <[email protected]>
VachaShah added a commit that referenced this pull request Jul 6, 2022
* Sort field in Hit deserializer should handle null events (#169)

Signed-off-by: Rene Cordier <[email protected]>
Signed-off-by: Matt Timmermans <[email protected]>

* Deprecating master terminology to support inclusive naming (#167)

* Deprecating master terminology to support inclusive naming

Signed-off-by: Vacha Shah <[email protected]>

* Fixing tests against unreleased OpenSearch

Signed-off-by: Vacha Shah <[email protected]>
Signed-off-by: Matt Timmermans <[email protected]>

* Fix issue 172

Signed-off-by: Matt Timmermans <[email protected]>

* Add test for issue 172

Signed-off-by: Matt Timmermans <[email protected]>

Co-authored-by: Rene Cordier <[email protected]>
Co-authored-by: Vacha Shah <[email protected]>
Co-authored-by: Matt Timmermans <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants