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] o.e.common.util #337

Merged

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Mar 16, 2021

This commit refactors the o.e.common.util package to the
o.opensearch.common.util namespace. All references throughout the codebase have
been refactored.

relates #160

@nknize nknize added >FORK Related to the fork process Rename Renaming to OpenSearch labels Mar 16, 2021
@odfe-release-bot
Copy link

✅   DCO Check Passed

@nknize nknize force-pushed the rename/server/common/util branch from dd41f90 to 192c23e Compare March 16, 2021 21:27
@odfe-release-bot
Copy link

✅   DCO Check Passed

@nknize nknize force-pushed the rename/server/common/util branch from 192c23e to d7c1f11 Compare March 16, 2021 22:17
@odfe-release-bot
Copy link

✅   DCO Check Passed

@nknize nknize force-pushed the rename/server/common/util branch from d7c1f11 to af579b9 Compare March 16, 2021 22:46
@odfe-release-bot
Copy link

✅   DCO Check Passed

@setiah setiah self-requested a review March 16, 2021 23:24
Copy link
Contributor

@setiah setiah left a comment

Choose a reason for hiding this comment

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

Mostly looks good. One minor comment/question

import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.util.concurrent.XRejectedExecutionHandler;
import org.opensearch.common.util.concurrent.EsRejectedExecutionException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this have changed to OpenSearchRejectedExecutionException as part of #305 (which is for server/src/main/java/org/opensearch/common)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see comment below. it was changed in name only, the class was never actually refactored.

@@ -32,14 +32,14 @@
import org.opensearch.common.unit.SizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.common.util.concurrent.OpenSearchRejectedExecutionException;
Copy link
Contributor

Choose a reason for hiding this comment

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

This was initially changed as part of #285. I'm tryin to gather which PR will be changing this to the rightful state

import org.opensearch.common.util.concurrent.OpenSearchRejectedExecutionException;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@harold-wang changed the import in name only. The class has not yet been refactored to this new name, that's why it appears in two different forms.

Copy link
Collaborator

Choose a reason for hiding this comment

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

😂 Really an interesting phenomenon. Thanks to Rabi that told me a ideal way to rename utilizing the refactoring in Intellij in PR # 319, I got the good idea. I realized we have done a few unprofessional work in the early days.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nknize @tlfeng I had quick talk with Rabi, using intelliJ refactor tool can avoid this. sorry that I did not realize this at the beginning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no worries @harold-wang ! we'll catch it in cleanup phase

@nknize nknize force-pushed the rename/server/common/util branch from af579b9 to f96c195 Compare March 17, 2021 03:27
@odfe-release-bot
Copy link

✅   DCO Check Passed

nknize added 2 commits March 16, 2021 22:32
This commit refactors the o.e.common.util package to the
o.opensearch.common.util namespace. All references throughout the codebase have
been refactored.

Signed-off-by: Nicholas Walter Knize <[email protected]>
Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize nknize force-pushed the rename/server/common/util branch from f96c195 to 2c5dd20 Compare March 17, 2021 03:32
@nknize nknize requested a review from setiah March 17, 2021 03:32
@odfe-release-bot
Copy link

✅   DCO Check Passed

Signed-off-by: Nicholas Walter Knize <[email protected]>
@odfe-release-bot
Copy link

✅   DCO Check Passed

@nknize nknize merged commit c654bda into opensearch-project:rename/opensearch Mar 17, 2021
nknize added a commit that referenced this pull request Mar 20, 2021
This commit refactors the o.e.common.util package to the
o.opensearch.common.util namespace. All references throughout the codebase have
been refactored.

Signed-off-by: Nicholas Walter Knize <[email protected]>
nknize added a commit that referenced this pull request Mar 22, 2021
This commit refactors the o.e.common.util package to the
o.opensearch.common.util namespace. All references throughout the codebase have
been refactored.

Signed-off-by: Nicholas Walter Knize <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>FORK Related to the fork process Rename Renaming to OpenSearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants