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

distinguish which labels belong to resource #58353

Merged
merged 1 commit into from Mar 21, 2018

Conversation

@juanvallejo
Member

juanvallejo commented Jan 16, 2018

Release note:

NONE

Usability improvement for kubectl label ... --list when listing labels for more than one resource.
Append resource kind/name before its set of labels.

Before

$ kubectl label dc myapp test-deployment-config label1=test --list
app=myapp
label1=test
label1=test

After

$ kubectl label dc myapp test-deployment-config label1=test --list
Listing labels for DeploymentConfig/myapp:
  label1=test
  app=myapp
Listing labels for DeploymentConfig/test-deployment-config:
  label1=test
for k, v := range accessor.GetLabels() {
fmt.Fprintf(o.out, "%s=%s\n", k, v)
fmt.Fprintf(o.out, " %s=%s\n", k, v)

This comment has been minimized.

@dixudx

dixudx Jan 17, 2018

Member

@juanvallejo Can you please paste an example output for before and after?

This comment has been minimized.

@juanvallejo

juanvallejo Jan 17, 2018

Member

Sure, will update PR description as well

Before

$ kubectl label dc myapp test-deployment-config label1=test --list
app=myapp
label1=test
label1=test

After

$ kubectl label dc myapp test-deployment-config label1=test --list
Listing labels for DeploymentConfig/myapp:
  label1=test
  app=myapp
Listing labels for DeploymentConfig/test-deployment-config:
  label1=test
@dixudx

This comment has been minimized.

Member

dixudx commented Jan 17, 2018

/retest

@dixudx

This comment has been minimized.

Member

dixudx commented Jan 18, 2018

/ok-to-test

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Jan 22, 2018

/test pull-kubernetes-e2e-kops-aws
cc @soltysh @shiywang

@soltysh

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 29, 2018

@soltysh

This comment has been minimized.

Contributor

soltysh commented Jan 29, 2018

/retest

@soltysh

This comment has been minimized.

Contributor

soltysh commented Jan 29, 2018

/assign @pwittrock
for approval

@deads2k

This comment has been minimized.

Contributor

deads2k commented Feb 1, 2018

It looks like this change makes the result unpipeable. How about only doing this if the result is a list? That way when the results are predicatable (when using a single result query), the results can be piped.

@@ -293,8 +293,9 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error {
return err
}
fmt.Printf("Listing labels for %s/%s:\n", info.Mapping.GroupVersionKind.Kind, info.Name)

This comment has been minimized.

@deads2k

deads2k Feb 1, 2018

Contributor

Tighten this to kind.group/name.

for k, v := range accessor.GetLabels() {
fmt.Fprintf(o.out, "%s=%s\n", k, v)
fmt.Fprintf(o.out, " %s=%s\n", k, v)

This comment has been minimized.

@deads2k

deads2k Feb 1, 2018

Contributor

only add the space when you have more than one. See the pipeability comment.

@k8s-ci-robot k8s-ci-robot added size/S and removed size/XS labels Feb 1, 2018

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Feb 1, 2018

@deads2k thanks, comments addressed

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Feb 9, 2018

/test pull-kubernetes-unit

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Feb 14, 2018

@deads2k or @pwittrock friendly ping

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 2, 2018

cc @soltysh for approval

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 5, 2018

/test pull-kubernetes-e2e-gce

2 similar comments
@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 5, 2018

/test pull-kubernetes-e2e-gce

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 5, 2018

/test pull-kubernetes-e2e-gce

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 5, 2018

@soltysh friendly ping

@soltysh soltysh added this to the v1.11 milestone Mar 16, 2018

@soltysh

/lgtm

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 20, 2018

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@derekwaynecarr @dixudx @dshulyak @juanvallejo @pwittrock @soltysh

Pull Request Labels
  • sig/cli: Pull Request will be escalated to these SIGs if needed.
  • priority/important-longterm: Escalate to the pull request owners; move out of the milestone after 1 attempt.
  • kind/feature: New functionality.
Help
@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 21, 2018

/retest

@soltysh

/lgtm
/approve no-issue

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Mar 21, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, juanvallejo, soltysh

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@juanvallejo

This comment has been minimized.

Member

juanvallejo commented Mar 21, 2018

/test pull-kubernetes-integration

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 21, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Mar 21, 2018

Automatic merge from submit-queue (batch tested with PRs 61487, 58353, 61078, 61219, 60792). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit 5286806 into kubernetes:master Mar 21, 2018

9 of 14 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-e2e-gce
Details
pull-kubernetes-e2e-gce Job triggered.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job triggered.
Details
pull-kubernetes-kubemark-e2e-gce Job triggered.
Details
pull-kubernetes-verify Job triggered.
Details
cla/linuxfoundation juanvallejo authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details

@juanvallejo juanvallejo deleted the juanvallejo:jvallejo/usability-fix-label branch Mar 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment