feat: add ports autocompletion for kubectl port-forward command#124683
Conversation
|
/retest |
|
I am able to test the new feature by:
https://github.com/kubernetes/kubectl/issues/1536 requested, to autocomplete If I edit the deployment to configure port Looks good to me. |
c424233 to
a81ce8b
Compare
|
/retest |
|
/lgtm Code has comments. |
|
@gnufred: changing LGTM is restricted to collaborators DetailsIn 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. |
|
What is the difference between this PR and #122752? |
I don't get your point exactly, but it was open for quite some time and there's no activity there. Also, you can see that there are some differences in terms of implementation + I have some tests here |
I think, in such cases it is better to notify the authors (e.g. #122752) that your PR is based on their code to prevent duplicate efforts and share the credits of the overall work. |
@ardaguclu I'm okay with @TessaIO taking over the implementation in this PR. I will close #122752 in favor of this PR. I appreciate it |
a81ce8b to
aa0c978
Compare
|
Completion working as expected. See comments about unit test. |
f3f2e32 to
e81b4aa
Compare
|
@brianpursley @ah8ad3 @ardaguclu Tests added. Feel free to do another round of review. |
|
/lgtm /cc @ardaguclu @brianpursley for review and approve |
|
@ah8ad3: GitHub didn't allow me to request PR reviews from the following users: for, review, and, approve. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn 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. |
|
LGTM label has been added. DetailsGit tree hash: e40e707393544dda6a0355b00fd8797ab4ac7bc7 |
ardaguclu
left a comment
There was a problem hiding this comment.
Actual code implementation looks good to me. Left a few comments for tests.
staging/src/k8s.io/kubectl/pkg/util/completion/completion_test.go
Outdated
Show resolved
Hide resolved
Signed-off-by: TessaIO <ahmedgrati1999@gmail.com>
e81b4aa to
a84e893
Compare
|
@TessaIO thanks for spending time and effort on this PR. LGTM |
|
/assign @brianpursley |
|
LGTM label has been added. DetailsGit tree hash: cb9c46516a5b6112406a65026baf4bde9060c6c2 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ah8ad3, brianpursley, TessaIO 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 |
|
/remove-sig auth |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR aims to add ports autocompletion for
kubectl port-forwardcommandWhich issue(s) this PR fixes:
Fixes kubernetes/kubectl#1536
Special notes for your reviewer:
For now, it just outputs the port number. If you want to support also port names let me know.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: