Skip to content

[improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-client-tools#22209

Merged
nodece merged 4 commits intoapache:masterfrom
nodece:pip-343-pulsar-client-tools
Mar 14, 2024
Merged

[improve][cli] PIP-343: Use picocli instead of jcommander in pulsar-client-tools#22209
nodece merged 4 commits intoapache:masterfrom
nodece:pip-343-pulsar-client-tools

Conversation

@nodece
Copy link
Copy Markdown
Member

@nodece nodece commented Mar 6, 2024

PIP: #22181

Motivation

Improve CLI user experience.

Due to the large scope of refactoring, I need to split the PR.

Modifications

  • Refactor the pulsar-client-tools code
    • pulsar-admin
    • pulsar-client
    • pulsar-shell
  • Move some converter from org.apache.pulsar.admin.cli.utils to org.apache.pulsar.admin.cli.utils.picocli
  • Fix some tests
  • validateValueWith is ignored, and not be supported by picocli
    • We should rely on the server to determine whether this value is valid, rather than at the CLI level.

Verifying this change

  • Make sure that the change passes the CI checks.

Relying on existing tests for validation.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Mar 6, 2024
@nodece nodece force-pushed the pip-343-pulsar-client-tools branch from f91d861 to bcf099f Compare March 6, 2024 07:47
@dao-jun dao-jun added this to the 3.3.0 milestone Mar 6, 2024
@nodece
Copy link
Copy Markdown
Member Author

nodece commented Mar 8, 2024

/pulsarbot rerun-failure-checks

1 similar comment
@nodece
Copy link
Copy Markdown
Member Author

nodece commented Mar 8, 2024

/pulsarbot rerun-failure-checks

Copy link
Copy Markdown
Contributor

@Technoboy- Technoboy- left a comment

Choose a reason for hiding this comment

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

LGTM
Test some methods and it works

Copy link
Copy Markdown
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

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

over all lgtm, just a few minor comments.

Comment thread pulsar-client-tools/src/main/java/org/apache/pulsar/client/cli/CmdProduce.java Outdated
@nodece nodece requested a review from dao-jun March 14, 2024 10:13
@nodece
Copy link
Copy Markdown
Member Author

nodece commented Mar 14, 2024

Ping @dao-jun

Copy link
Copy Markdown
Member

@dao-jun dao-jun left a comment

Choose a reason for hiding this comment

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

I've tried some cmds, LGTM

@nodece nodece force-pushed the pip-343-pulsar-client-tools branch from 707bfdb to 96ab103 Compare March 14, 2024 13:48
@nodece
Copy link
Copy Markdown
Member Author

nodece commented Mar 14, 2024

/pulsarbot rerun-failure-checks

@nodece nodece merged commit 434ec1b into apache:master Mar 14, 2024
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
…lient-tools (apache#22209)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants