You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Joo Hyuk Kim <be...@gmail.com> on 2023/06/30 07:20:11 UTC

[DISCUSS] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter

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

Re: [DISCUSS] PIP-280 : Refactor CLI Argument Parsing Logic for Measurement Units using JCommander's custom converter

Posted by tison <wa...@gmail.com>.
I read the proposal on GitHub and it's clear to reduce code duplication
with a middle size refactor. Although I think it may not require a PIP, we
can try to proceed in the PIP process and see if the PIP process fits this
proposal.

cc developers in this domain AFAIK.

Best,
tison.


Joo Hyuk Kim <be...@gmail.com> 于2023年6月30日周五 15:20写道:

> 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
>