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

Rename everything from opensearch-sdk to opensearch-sdk-java #96

Merged
merged 16 commits into from
Aug 23, 2022
Merged

Rename everything from opensearch-sdk to opensearch-sdk-java #96

merged 16 commits into from
Aug 23, 2022

Conversation

mloufra
Copy link
Contributor

@mloufra mloufra commented Aug 17, 2022

Description

The Repo name of opensearch-sdk has been changed to opensearch-sdk-java, so the namings and description in the following files need to be synchronized.
(CONTRIBUTING.md, DESIGN.md, DEVELOPER_GUIDE.md, MAINTAINERS.md, README.md, SECURITY.md, THIRD-PARTY-NOTICES, settings.gradle and CODEOWNERS)

Issues Resolved

#13

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.

@mloufra mloufra requested a review from a team August 17, 2022 19:58
@@ -1,2 +1,2 @@
# This should match the owning team set up in https://github.com/orgs/opensearch-project/teams
* @opensearch-project/opensearch-sdk
* @opensearch-project/opensearch-sdk-java
Copy link
Member

Choose a reason for hiding this comment

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

I did change the name of the team from opensearch-sdk to opensearch-sdk-java

owaiskazi19
owaiskazi19 previously approved these changes Aug 17, 2022
Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

LGTM!

CONTRIBUTING.md Outdated Show resolved Hide resolved
@owaiskazi19 owaiskazi19 dismissed their stale review August 17, 2022 20:24

Need changes

Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Please change JAVA to Java throughout, plus the following:

CONTRIBUTING.md Outdated
@@ -68,7 +68,7 @@ By making a contribution to this project, I certify that:
consistent with this project or the open source license(s)
involved.
```
We require that every contribution to OpenSearch-SDK is signed with a Developer Certificate of Origin. Additionally, please use your real name. We do not accept anonymous contributors nor those utilizing pseudonyms.
We require that every contribution to OpenSearch-SDK-JAVA is signed with a Developer Certificate of Origin. Additionally, please use your real name. We do not accept anonymous contributors nor those utilizing pseudonyms.
Copy link
Member

Choose a reason for hiding this comment

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

"Developer Certificate of Origin" isn't clear to new contributors. This needs to be linked to better documentation.

I'd suggest linking to the main project CONTRIBUTING doc:
https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin

DESIGN.md Outdated
@@ -41,8 +41,8 @@ Here is an example extension configuration `extensions.yml`:

```
extensions:
- name: opensearch-sdk // extension name
uniqueId: opensearch-sdk-1 // identifier for the extension
- name: opensearch-sdk-java // extension name
Copy link
Member

Choose a reason for hiding this comment

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

Can we please make this sample-extension? Extension names are not and never should be this project's name. I have a request in #74 to change this, and changing it here will avoid creating a merge conflict that I'll need to handle. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -165,7 +165,7 @@ Run tests :
./gradlew clean build test
```
## Generate Artifact
In opensearch-sdk navigate to build/distributions. Look for tar ball in the form `opensearch-sdk-1.0.0-SNAPSHOT.tar`. If not found follow the below steps to create one:
In opensearch-sdk-java navigate to build/distributions. Look for tar ball in the form `opensearch-sdk-1.0.0-SNAPSHOT.tar`. If not found follow the below steps to create one:
Copy link
Member

Choose a reason for hiding this comment

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

should be a -java in the tarball name.

DEVELOPER_GUIDE.md Show resolved Hide resolved
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

We typically lowercase Java like so. A few places where we say JAVA.

README.md Outdated
@@ -1,4 +1,4 @@
# OpenSearch SDK
# OpenSearch SDK JAVA
Copy link
Member

Choose a reason for hiding this comment

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

No need to SHOUT :) Java

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -102,7 +102,7 @@ During OpenSearch bootstrap, `ExtensionsOrchestrator` will then discover the ext
[2022-06-16T21:30:19,000][INFO ][o.o.e.ExtensionsOrchestrator] [runTask-0] received PluginResponse{examplepluginname}
```

OpenSearch SDK terminal will also log all requests and responses it receives from OpenSearch :
OpenSearch SDK JAVA terminal will also log all requests and responses it receives from OpenSearch :
Copy link
Member

Choose a reason for hiding this comment

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

Java

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I think the product is called "OpenSearch SDK for Java" (note the for that disambiguates that this is a non-technical term), while the repo is called "opensearch-sdk-java" and the namespace is "org.opensearch.sdk". I propose to align along those lines?

dbwiddis
dbwiddis previously approved these changes Aug 19, 2022
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

LGTM. Would still like a link to DCO but that can be handled in another PR.

Question for @saratvemulapalli and @owaiskazi19 -- should we be aligning the package name to include the .java?

@mloufra
Copy link
Contributor Author

mloufra commented Aug 19, 2022

I think the product is called "OpenSearch SDK for Java" (note the for that disambiguates that this is a non-technical term), while the repo is called "opensearch-sdk-java" and the namespace is "org.opensearch.sdk". I propose to align along those lines?

Thank you for @dblock your suggestion.
How about let's hold a vote on whether we need to make changes on namespace"org.opensearch.sdk". @dbwiddis @owaiskazi19 @saratvemulapalli

@mloufra mloufra changed the title Issue #13 Rename opensearch sdk Rename everything from opensearch-sdk to opensearch-sdk-java Aug 19, 2022
@owaiskazi19
Copy link
Member

I'm fine with keeping it org.opensearch.sdk.java as other languages SDK's can also follow the same namespace. Let's see what others have to say.

@dbwiddis
Copy link
Member

I'm fine with keeping it org.opensearch.sdk.java

But it's not presently .java so we are not "keeping" it. It is currently org.opensearch.sdk.

I would prefer to keep it as-is (for now) as from a Java perspective, namespace and package are the same thing and we will not conflict with namespaces in other languages.

as other languages SDK's can also follow the same namespace.

By this I assume you mean a C# SDK would also use an org.opensearch.sdk namespace and this would not conflict, as the imports are from entirely different ecosystems. Similar for other languages. The only problem might be languages which use the Java ecosystem such as Kotlin.

This is not necessarily a "permanent" choice. If we find a need to separate java-specific stuff from the SDK we can reconsider it then.

@owaiskazi19
Copy link
Member

owaiskazi19 commented Aug 22, 2022

But it's not presently .java so we are not "keeping" it. It is currently org.opensearch.sdk.

My bad I didn't frame it properly. I meant we can change the namespace to org.opensearch.sdk.java.

By this I assume you mean a C# SDK would also use an org.opensearch.sdk namespace and this would not conflict, as the imports are from entirely different ecosystems. Similar for other languages. The only problem might be languages which use the Java ecosystem such as Kotlin.

Yes, I was thinking about Kotlin too and that's why thought of having the language name in the namespace.

This is not necessarily a "permanent" choice. If we find a need to separate java-specific stuff from the SDK we can reconsider it then.

Sounds good to me. We can keep the namespace org.opensearch.sdk for now and open an issue to reconsider or have the discussion in the future for the same. @mloufra can you open an issue?

@mloufra
Copy link
Contributor Author

mloufra commented Aug 22, 2022

Sounds good to me. We can keep the namespace org.opensearch.sdk for now and open an issue to reconsider or have the discussion in the future for the same. @mloufra can you open an issue?

New issue has been created.
#99

owaiskazi19
owaiskazi19 previously approved these changes Aug 22, 2022
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Still some changes previously discussed.

DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
DEVELOPER_GUIDE.md Outdated Show resolved Hide resolved
@mloufra mloufra requested a review from dbwiddis August 23, 2022 16:37
@owaiskazi19 owaiskazi19 merged commit 9e60bea into opensearch-project:main Aug 23, 2022
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
…rch-project#96)

* issue opensearch-project#28

Signed-off-by: mloufra <[email protected]>

* Update the lastest coomit

Signed-off-by: mloufra <[email protected]>

* fix merge conflict

Signed-off-by: mloufra <[email protected]>

* Rename the method and fix the conflict

Signed-off-by: mloufra <[email protected]>

* Rename everything from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

* Rename everything from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

* Rename everythingg from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

* Rename extensionName to avoid merge conflict

Signed-off-by: mloufra <[email protected]>

* Renanme everything from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

* Rename everything from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

* Rename everything from opensearch-sdk to opensearch-sdk-java

Signed-off-by: mloufra <[email protected]>

Signed-off-by: mloufra <[email protected]>
Signed-off-by: Frank Lou <[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