📖 Tidy up completion.md#5103
Conversation
The committers listed above are authorized under a signed CLA. |
|
Welcome @abenn135! |
|
Hi @abenn135. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. DetailsInstructions 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. |
e304e21 to
c56da72
Compare
|
(CLA request is pending with my organization) |
vitorfloriano
left a comment
There was a problem hiding this comment.
Hi @abenn135 , thanks for working on this! The instructions for Bash completions are much clearer now. I noticed a couple of things that might help the PR move forward more smoothly:
- All PRs need to be squashed into a single commit to keep the history clean.
- Using a conventional commit title/message for consistency.
Thanks again for improving the docs. This is a nice quality-of-life change. :)
| GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu) | ||
| Copyright (C) 2020 Free Software Foundation, Inc. | ||
| License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> | ||
|
|
||
| This is free software; you are free to change and redistribute it. | ||
| There is NO WARRANTY, to the extent permitted by law. |
There was a problem hiding this comment.
Does this info add any value? Couldn't we just have the command and leave out the output? WDYT?
There was a problem hiding this comment.
Sure, I can remove it. Done.
| ```bash | ||
| cat /etc/shells | grep '^.*/bash' | ||
| ``` |
There was a problem hiding this comment.
NIT: I noticed the codeblocks added in this PR have a different identation than the other codeblocks already present in the file. Is there a reason for that?
There was a problem hiding this comment.
This indent keeps them within the containing bullet points. So instead of
- Blah blah
<code at top level indent, breaking bulleted list>
- disconnected bullet
you get
-
blah blah
<code contained within bullet with paragraph break> -
Next bullet continues bulleted list
There was a problem hiding this comment.
Oh I see it now 🤔 I hadn't looked at the preview of the file. Yeah, it looks better indented that way.
| ## Zsh | ||
|
|
||
| </aside> | ||
| Follow a similar protocol for `zsh` completion. | ||
|
|
||
| <aside class="note"> | ||
| <h1>Fish</h1> | ||
| ## Fish | ||
|
|
||
| ``` | ||
| source (kubebuilder completion fish | psub) | ||
|
|
||
| </aside> | ||
| ``` |
There was a problem hiding this comment.
Could we take this opportunity and also expand on the instructions for Zsh and Fish? Just a suggestion. WDYT?
There was a problem hiding this comment.
I don't have zsh nor fish installed, so I wouldn't be able to test those commands unfortunately.
c56da72 to
e3fd1f1
Compare
…etc/shells removes all other shells instead of simply adding another shell as an option.
e3fd1f1 to
83fc3b3
Compare
abenn135
left a comment
There was a problem hiding this comment.
Please take another look :)
| GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu) | ||
| Copyright (C) 2020 Free Software Foundation, Inc. | ||
| License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> | ||
|
|
||
| This is free software; you are free to change and redistribute it. | ||
| There is NO WARRANTY, to the extent permitted by law. |
There was a problem hiding this comment.
Sure, I can remove it. Done.
| ```bash | ||
| cat /etc/shells | grep '^.*/bash' | ||
| ``` |
There was a problem hiding this comment.
This indent keeps them within the containing bullet points. So instead of
- Blah blah
<code at top level indent, breaking bulleted list>
- disconnected bullet
you get
-
blah blah
<code contained within bullet with paragraph break> -
Next bullet continues bulleted list
| ## Zsh | ||
|
|
||
| </aside> | ||
| Follow a similar protocol for `zsh` completion. | ||
|
|
||
| <aside class="note"> | ||
| <h1>Fish</h1> | ||
| ## Fish | ||
|
|
||
| ``` | ||
| source (kubebuilder completion fish | psub) | ||
|
|
||
| </aside> | ||
| ``` |
There was a problem hiding this comment.
I don't have zsh nor fish installed, so I wouldn't be able to test those commands unfortunately.
camilamacedo86
left a comment
There was a problem hiding this comment.
Thank you for the contirbution 🥇
/lgtm
/ok-to-test
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abenn135, camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Tidy up completion.md. The doc as written is confusing and has a few issues. For example, following the instruction:
echo “/usr/local/bin/bash” > /etc/shells(a) has the wrong kind of double quotes, and (b) blows away all your other shells instead of simply adding
/usr/local/bin/bashto the list. This PR cleans that doc up a bit.