Skip to content

HDDS-6514. Cleanup constructors and builders in OM helpers#3244

Closed
kaijchen wants to merge 11 commits intoapache:masterfrom
kaijchen:HDDS-6514
Closed

HDDS-6514. Cleanup constructors and builders in OM helpers#3244
kaijchen wants to merge 11 commits intoapache:masterfrom
kaijchen:HDDS-6514

Conversation

@kaijchen
Copy link
Member

@kaijchen kaijchen commented Mar 25, 2022

What changes were proposed in this pull request?

Use builder pattern to avoid constructor telescoping in OM helpers.
And reduce parameter numbers in constructors, thus removing the need of @SuppressWarnings("checkstyle:ParameterNumber")

What is the link to the Apache JIRA

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

How was this patch tested?

Unit test

@guihecheng
Copy link
Contributor

We could combine HDDS-6513 and this into one since they are doing the same cleanups to source files under the same directory.

@kaijchen
Copy link
Member Author

We could combine HDDS-6513 and this into one since they are doing the same cleanups to source files under the same directory.

There are many classes with the same problem, inside om.helpers or outside.
I prefer to keep the patch small and easy to review.

@guihecheng
Copy link
Contributor

Oh if there are many(more than these 2) such problems and you've got a plan to fix them all, then we'd better have a base JIRA for "Avoid constructor for some submodule" and create sub-tasks under it, then we could know how many problems have fixed and how many left rather than randomly fix them one by one.

@kaijchen
Copy link
Member Author

kaijchen commented Mar 28, 2022

Oh if there are many(more than these 2) such problems and you've got a plan to fix them all, then we'd better have a base JIRA for "Avoid constructor for some submodule" and create sub-tasks under it, then we could know how many problems have fixed and how many left rather than randomly fix them one by one.

Thanks for the advice. I have changed HDDS-6513 to track them.

@kaijchen kaijchen marked this pull request as draft March 29, 2022 03:02
@kaijchen kaijchen changed the title HDDS-6514. Avoid constructor telescoping in OmKeyInfo HDDS-6514. Avoid constructor telescoping in OM helpers Mar 29, 2022
@kaijchen kaijchen force-pushed the HDDS-6514 branch 3 times, most recently from a98fa81 to 372eaa2 Compare March 29, 2022 11:51
@kaijchen kaijchen changed the title HDDS-6514. Avoid constructor telescoping in OM helpers HDDS-6514. Cleanup constructors and builders in OM helpers Mar 29, 2022
@kaijchen kaijchen force-pushed the HDDS-6514 branch 3 times, most recently from 13e38f2 to 1989dac Compare March 30, 2022 03:01
@kaijchen kaijchen marked this pull request as ready for review March 30, 2022 03:02
@kaijchen
Copy link
Member Author

There are many classes with the same problem, inside om.helpers or outside.
I prefer to keep the patch small and easy to review.

I decided to group them to reduce the number of pull requests.

@kaijchen
Copy link
Member Author

Rebased to master to solve the merge conflicts.

@kaijchen kaijchen closed this May 25, 2022
@kaijchen kaijchen deleted the HDDS-6514 branch August 2, 2022 03:46
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.

2 participants