Skip to content
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

Remove duplicate get errs #37270

Merged

Conversation

xilabao
Copy link
Contributor

@xilabao xilabao commented Nov 22, 2016

old:

$ kubectl get ns
NAME          STATUS    AGE
default       Active    2m
kube-system   Active    2m

$ kubectl get ns --all-namespaces
NAMESPACE   NAME      STATUS    AGE
error: namespace is not namespaced
error: namespace is not namespaced

new:

$ kubectl get ns --all-namespaces
NAMESPACE   NAME      STATUS    AGE
error: namespace is not namespaced

This change is Reviewable

@k8s-ci-robot
Copy link
Contributor

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands will still work. Regular contributors should join the org to skip this step.

If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 22, 2016
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Nov 22, 2016
@deads2k
Copy link
Contributor

deads2k commented Nov 22, 2016

@fabianofranz for experience.

@deads2k deads2k assigned fabianofranz and unassigned deads2k Nov 22, 2016
@fabianofranz
Copy link
Contributor

@k8s-bot ok to test

@xilabao
Copy link
Contributor Author

xilabao commented Nov 29, 2016

@fabianofranz PTAL

@fabianofranz
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 29, 2016
@k8s-github-robot k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 29, 2016
@fabianofranz
Copy link
Contributor

@ncdc needs a help adding release-note-none.

@deads2k deads2k added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Nov 29, 2016
@deads2k
Copy link
Contributor

deads2k commented Nov 29, 2016

@ncdc needs a help adding release-note-none.

You can lgtm, but not set a release-note?

@fabianofranz
Copy link
Contributor

You can lgtm, but not set a release-note?

Correct, which makes my lgtm pretty much useless. Hopefully it's close to being addressed: kubernetes-retired/contrib#1908 (comment).

@fabianofranz
Copy link
Contributor

@ncdc Thanks, also needs to remove do-not-merge please.

@fabianofranz
Copy link
Contributor

actually @deads2k ^

@k8s-github-robot
Copy link

@k8s-bot test this issue: #IGNORE

Tests have been pending for 24 hours

@ncdc ncdc removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 29, 2016
@ncdc ncdc added this to the v1.6 milestone Nov 29, 2016
@k8s-github-robot
Copy link

@k8s-bot test this issue: #IGNORE

Tests have been pending for 24 hours

@xilabao
Copy link
Contributor Author

xilabao commented Dec 5, 2016

@k8s-bot gci gce e2e test this

@k8s-ci-robot
Copy link
Contributor

@xilabao: you can't request testing unless you are a kubernetes member.

In response to this comment:

@k8s-bot gci gce e2e test this

If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository.

@@ -464,6 +470,10 @@ func RunGet(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args [
filteredResourceCount++
continue
}
if errs.Has(err.Error()) {
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement changes behavior -- previously, errors from filterfuncs would not block printerobj handling.

I think you want:

if !errs.Has(err.Error()) {
    errs.Insert(err.Error())
    allErrs = append(allErrs, err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 8, 2016
@xilabao
Copy link
Contributor Author

xilabao commented Dec 9, 2016

@fabianofranz PTAL

@fabianofranz
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 9, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 0dc166a. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit 0dc166a. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@rmmh
Copy link
Contributor

rmmh commented Dec 9, 2016

Look like generic timeouts?

@k8s-bot gci gce e2e test this
@k8s-bot gce etcd3 e2e test this

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 37270, 38309, 37568, 34554)

@k8s-github-robot k8s-github-robot merged commit 68b17c2 into kubernetes:master Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants