> As long as we don't introduce any breaking change and the new parameters > are covered by unit test
Made a commit(Link [1]) to improve considerations Link [1] https://github.com/apache/pulsar/pull/20691/commits/96515b85e97a56133a512165c981361fab939ec1 Thanks, JooHyukKim (Vince) On Tue, Jul 11, 2023 at 6:58 PM Joo Hyuk Kim <beansk...@gmail.com> wrote: > > As long as we don't introduce any breaking change and the new parameters > are covered by unit test > > Hello, thank you for your feedback. > I agree and will add following to PIP file. > "The refactored parameters should maintain coverage. " > > On Tue, Jul 11, 2023 at 6:34 PM Nicolò Boschi <boschi1...@gmail.com> > wrote: > >> +1 binding >> As long as we don't introduce any breaking change and the new parameters >> are covered by unit test >> >> Thanks, >> Nicolò Boschi >> >> >> Il giorno mar 11 lug 2023 alle ore 05:00 Qiang Zhao < >> mattisonc...@apache.org> >> ha scritto: >> >> > +1(binding) >> > >> > Best, >> > Mattison >> > >> > On 2023/07/07 09:25:22 Joo Hyuk Kim wrote: >> > > Hi community, >> > > >> > > This PIP has received a couple of approvals in github PR link [1] >> > > So I thought it's time to vote. >> > > >> > > ## Motivation >> > > >> > > In the current Pulsar codebase, the logic to parse CLI arguments for >> > > measurement units like time and bytes is >> > > >> > > scattered across various CLI classes. Each value read has its distinct >> > > parsing implementation, leading to a lack of code >> > > >> > > reuse. >> > > >> > > >> > > ## Goals >> > > >> > > >> > > This PIP is to refactor the argument parsing logic to leverage the >> > > `@Parameter.converter` >> > > >> > > functionality provided by JCommander [link 3]. This will isolate the >> > > measurement-specific parsing logic and increase >> > > >> > > code >> > > >> > > reusability. >> > > >> > > >> > > ### In Scope >> > > >> > > >> > > - Refactor all `Cmd` classes to utilize the converter functionality of >> > > JCommander. This will streamline the parsing >> > > >> > > logic and simplify the codebase. >> > > >> > > - Refer to bottom section "Concrete Example", before "Links" >> > > >> > > - Or on-going PR with small use case in >> > > https://github.com/apache/pulsar/pull/20663 >> > > >> > > >> > > ## links >> > > >> > > >> > > [1] PR : https://github.com/apache/pulsar/pull/20691 >> > > >> > > >> > > >> > > Best regards, >> > > >> > > JooHyukKim (Vince) >> > > >> > >> >
