Hi, community With regards to the refactor work, I made a PR [1] that shows how Command classes will be improved. Any feedback on the PR or the PIP [2] itself will be much appreciated.
Links [1] PR : https://github.com/JooHyukKim/pulsar/pull/15 [2] PIP : https://github.com/apache/pulsar/pull/20691 Best, joo hyuk (vince) On 2023/06/30 07:20:11 Joo Hyuk Kim wrote: > Hi, community > > > I am proposing to refactor `Cmd*` classes’s measurement parsing logic. > > > ## Background > > > As it stands, the parsing logic for measurement units such as time and > bytes in various CLI classes is implemented verbosely, contained in the Cmd > converter themselves. Leading to a lack of code reuse and possible > inconsistencies. > > > ## Proposal > > > The idea is to refactor all `Cmd` classes to utilize the converter > functionality of JCommander (link[1]). This will allow us to isolate the > measurement-specific parsing logic and streamline the code, thereby > improving maintainability and reusability. See the concrete example in this > PIP for a more in-depth explanation. > > > There is ongoing PR to provide concept in (link [2]) > > > ## Concerns > > > 1. Will this create too much work or introduce confusion? > > > By working class-by-class or inner-class basis, we can gradually refactor > the codebase without causing disruption. The changes are expected to > increase readability and maintainability, making future modifications > easier and less error-prone. > > > 2. Will it affect existing functionality? > > > The proposed changes are meant to streamline and standardize the code > without altering functionality. > > > ## Implementation > > > The initial focus will be on the following classes: > > > - `CmdNamespaces.java` > > - `CmdTopics.java` > > - `CmdTopicPolicies.java` > > > More classes may be added as the refactoring process continues. > > > Let me know what you think. > > > Best, > > JooHyuk > > > PIP : https://github.com/apache/pulsar/pull/20691 > > > Links : > > [1] https://jcommander.org/#_custom_types_converters_and_splitters > > [2] https://github.com/apache/pulsar/pull/20663 >
