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.

Sign up for GitHub

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

Jump to bottom

refactor(cli): Improve CLI commands with better error handling and output formatting #6573

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

refactor(cli): Improve CLI commands with better error handling and output formatting #6573

wants to merge 10 commits into from

Conversation

Brijeshthummar02
Copy link
Contributor

What changes were proposed in this pull request?

This PR enhances multiple CLI commands by improving error handling, resource management, and output formatting. The following changes were made:

  • Improved exception handling for better debugging and user feedback.
  • Enhanced error messages for missing entities to provide clearer guidance.
  • Implemented try-with-resources for better resource management where applicable.
  • Ensured proper handling of empty responses and formatted outputs for better readability.
  • Standardized approach to handling CLI command execution errors.

Why are the changes needed?

These improvements ensure that CLI commands provide more reliable feedback to users, prevent unhandled exceptions, and improve resource management. The changes also enhance maintainability and debugging for developers.

Fix: #6528

Does this PR introduce any user-facing change?

Yes, the following user-facing changes are introduced:

  • Improved error messages for missing entities.
  • Better formatting for command outputs.
  • Safer resource handling to prevent unexpected failures.

@Brijeshthummar02
Copy link
Contributor Author

Brijeshthummar02 commented Feb 28, 2025 •
edited
Loading

@justinmclean it was taking too long for me as there where multiple files but finally made my way, now it's up to you, you may go through the changes and feel free to mention also added comments where i feel it should be there for better readability.

@Brijeshthummar02
Copy link
Contributor Author

@yuqi1129 need your review on this PR.

@yuqi1129
Copy link
Contributor

@yuqi1129 need your review on this PR.

Got

@Brijeshthummar02
Copy link
Contributor Author

@yuqi1129 That was too quick, Thanks!!

@Brijeshthummar02
Copy link
Contributor Author

@yuqi1129 need your review on this PR again

@Brijeshthummar02
Copy link
Contributor Author

@justinmclean @tengqm tried to fix and resolve as suggested can just go through it ones also removed what i feel is not needed.

@Brijeshthummar02 Brijeshthummar02 requested a review from tengqm March 2, 2025 06:16
@Brijeshthummar02
Copy link
Contributor Author

@justinmclean tried to fix all return statement, using it was my best practice but for this repo as u said we don't generally need so removed apart from that fixed all if statements outside the try block. Also thankyouuu! for giving reviews always as there are multiples files so this is taking way more time.

@Brijeshthummar02
Copy link
Contributor Author

@tengqm made relevant changes.

@Brijeshthummar02 Brijeshthummar02 requested a review from tengqm March 2, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinmclean justinmclean Awaiting requested review from justinmclean

@tengqm tengqm Awaiting requested review from tengqm

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Improvement] Fix potential null pointer expections in CLI
4 participants
@Brijeshthummar02 @yuqi1129 @justinmclean @tengqm