-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix: Report any unknown options found on the command line. #159
Conversation
Can one of the admins verify this patch? |
/packit build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I have few requests and suggestions.
Thank you for the review input @jirihnidek! It's very much appreciated. I actually reflected on this after I submitted it and realized I should dry up the code. Your suggestions are spot on. I believe I have addressed them in commit: af612bd and retested the following:
|
Card ID: CCT-524 The rhc subcommands, connect, disconnect and status ignore any unknown options given. This PR will fix this by returning an error when any unknown options are found.
af612bd
to
8689826
Compare
@jirihnidek I believe I've addressed all of the changes you have requested. Please let me know what you think now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for updates 👍
/packit build |
Thank you for the help with this @jirihnidek |
Card ID: CCT-524
The rhc subcommands, connect, disconnect and status ignore any unknown options given. This PR will fix this by returning an error when any unknown options are found.
The following tests were performed:
Test 1: One unknown argument give.
Test 2: Valid arguments given but unknown arguments were also given. The extra unknown
arguments will produce an error.