Skip to content

HDDS-7301. Cleanup of org.apache.hadoop.ozone.OmUtils#3812

Merged
adoroszlai merged 2 commits intoapache:masterfrom
myskov:refactor_omutils
Feb 18, 2023
Merged

HDDS-7301. Cleanup of org.apache.hadoop.ozone.OmUtils#3812
adoroszlai merged 2 commits intoapache:masterfrom
myskov:refactor_omutils

Conversation

@myskov
Copy link
Contributor

@myskov myskov commented Oct 8, 2022

What changes were proposed in this pull request?

Removed dead code and code smells reported by Sonar.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-7301

How was this patch tested?

unit tests

Copy link
Member

@kaijchen kaijchen left a comment

Choose a reason for hiding this comment

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

The change is correct, however, some public static methods such as getOmRpcPort might be useful in the future (they are used only once and the patch has inlined them). What do you think @adoroszlai?

@kaijchen kaijchen requested a review from adoroszlai October 9, 2022 04:01
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @myskov for the patch.

some public static methods such as getOmRpcPort might be useful in the future (they are used only once and the patch has inlined them)

Not only that, but it is possible that user code may call these public methods. I think it's better to deprecate them first, to be removed in some future release.

@DaveTeng0
Copy link
Contributor

Thanks @myskov for the patch.

some public static methods such as getOmRpcPort might be useful in the future (they are used only once and the patch has inlined them)

Not only that, but it is possible that user code may call these public methods. I think it's better to deprecate them first, to be removed in some future release.

hey @myskov ~ would you mind marking those methods deprecated for now and creating ticket(s) for the removal in the future?

@adoroszlai adoroszlai merged commit 59c87ba into apache:master Feb 18, 2023
@adoroszlai
Copy link
Contributor

Thanks @myskov for the patch, @kaijchen for the review.

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