Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CASSANDRA-17445 switch management framework from airline to picocli #2497

Open
wants to merge 20 commits into
base: trunk
Choose a base branch
from

Conversation

Mmuzaf
Copy link
Contributor

@Mmuzaf Mmuzaf commented Jul 18, 2023

Thanks for sending a pull request! Here are some tips if you're new here:

  • Ensure you have added or run the appropriate tests for your PR.
  • Be sure to keep the PR description updated to reflect all changes.
  • Write your PR title to summarize what this PR proposes.
  • If possible, provide a concise example to reproduce the issue for a faster review.
  • Read our contributor guidelines
  • If you're making a documentation change, see our guide to documentation contribution

Commit messages should follow the following format:

<One sentence description, usually Jira title or CHANGES.txt summary>

<Optional lengthier description (context on patch)>

patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####

Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>

The Cassandra Jira

@Mmuzaf
Copy link
Contributor Author

Mmuzaf commented Jun 28, 2024

image

@maedhroz maedhroz self-requested a review July 2, 2024 22:32
@maedhroz
Copy link
Contributor

maedhroz commented Jul 8, 2024

@Mmuzaf btw, the title here mentions airline-2, but we're actually going to picocli, right?

@Mmuzaf Mmuzaf changed the title CASSANDRA-17445 switch management framework from airline to airline-2 CASSANDRA-17445 switch management framework from airline to picocli Jul 8, 2024
@Mmuzaf
Copy link
Contributor Author

Mmuzaf commented Jul 8, 2024

@Mmuzaf btw, the title here mentions airline-2, but we're actually going to picocli, right?

@maedhroz Correct, I've updated the title.
Airline-2 doesn't work well for us because it doesn't allow the jmx options to be separated from the commands (don't want to include jmx options in the command hierarchy as they are related to the nodetool only).

/**
* Base class for all nodetool commands.
*/
public abstract class BaseCommand implements Runnable
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename this to AbstractCommand

protected Output logger;

/** The ServiceBridge instance to interact with the Cassandra node. */
protected ServiceMBeanBridge bridge;
Copy link
Contributor

Choose a reason for hiding this comment

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

why protected when we have setter / getter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants