-
Notifications
You must be signed in to change notification settings - Fork 1k
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
MINOR: Cause 'ksql help' and 'ksql -help' to behave the same as 'ksql… #2023
Conversation
It looks like @vcrfxia hasn't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here. Once you've signed reply with Appreciation of efforts, clabot |
[clabot:check]
…On Mon, Oct 8, 2018 at 11:59 AM Confluent CLA Bot ***@***.***> wrote:
It looks like @vcrfxia <https://github.com/vcrfxia> hasn't signed our *C*ontributor
*L*icense *A*greement, yet.
The purpose of a CLA is to ensure that the guardian of a project's outputs
has the necessary ownership or grants of rights over all contributions to
allow them to distribute under the chosen licence.
Wikipedia <http://en.wikipedia.org/wiki/Contributor_License_Agreement>
You can read and sign our full Contributor License Agreement here
<http://clabot.confluent.io/cla>.
Once you've signed reply with [clabot:check] to prove it.
Appreciation of efforts,
clabot
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2023 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQuGO0-0Fab9E1LlHBEBBRTcurJONhm5ks5ui6B8gaJpZM4XNfrK>
.
|
thanks @vcrfxia ! The changes look fine, but I am curious about how you tested this change. |
Besides checking that integration tests continue to pass, I verified that "ksql", "ksql-server-start", and "ksql-datagen" (the only three scripts affected by changes to "ksql-run-class") are as expected when run with each of the help flags. Are there other tests/checks I should perform? Thanks! |
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 @vcrfxia It would be good to add those details to the "Testing done" section. |
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! Feel free to merge once the branch is up to date and the build passes.
… -h' and 'ksql --help'
… -h' and 'ksql --help'
Description
What behavior do you want to change, why, how does your patch achieve the changes?
Fixes #1444, by redirecting 'ksql help' and 'ksql -help' to 'ksql --help'.
Testing done
Describe the testing strategy. Unit and integration tests are expected for any behavior changes.
Verified that "ksql", "ksql-server-start", and "ksql-datagen" (the only three scripts affected by changes to "ksql-run-class") are as expected when run with each of the help flags.
Reviewer checklist