-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Add examples to etcdctl snapshot
command's help
#18183
Conversation
Hi @ah8ad3. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Im not sure about this test https://github.com/etcd-io/etcd/actions/runs/9534220896/job/26278395488?pr=18183 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #18183 +/- ##
=============================
=============================
Continue to review full report in Codecov by Sentry.
|
I think you need to run a |
9ab112f
to
8fc2134
Compare
I see what you mean, thanks that makes sense. I updated the code hopefully that will fix it. |
Speaking of moving the file from |
/retitle Add examples to |
etcdctl snapshot
command's help
As i understand this help file is only for |
/remove-label area/* |
@ah8ad3: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Sorry, always happens to me when i rebase with wrong repository(forked one), tried to fix it. |
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 addressing James' comment. I think this is going in the right direction. I left a couple of comments.
c3a384c
to
e232b59
Compare
etcdctl util, create normalizer of examples in util, add some more examples, change indent of etcdctl from "\t" to " " Signed-off-by: ah8ad3 <ah8ad3@gmail.com>
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 @ah8ad3
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 - New help examples look great, thanks @ah8ad3
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, @ah8ad3 :)
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ah8ad3, ahrtr, ivanvc, jmhbnz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Part of #17777.
Add example section into
etcdctl
by first adding it to helper function of cobra.Create a
util
module intoetcdctl
move helper there.Add a normalizer function for exmaples.
Change indent of
etcdctl
from"\t
" to" "
.I tried to keep this PR simple and add only functionality of examples to helper and add examples of snapshot so it can be easy to review.
/cc @ivanvc @jmhbnz