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.

Sign up for GitHub

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

Jump to bottom

Print recognized file extensions in resource builder error #13482

Merged
merged 1 commit into from
Sep 4, 2015
Merged

Print recognized file extensions in resource builder error #13482

merged 1 commit into from
Sep 4, 2015

Conversation

j3ffml
Copy link
Contributor

@j3ffml j3ffml commented Sep 1, 2015

closes #13450

@j3ffml
Copy link
Contributor Author

j3ffml commented Sep 1, 2015

@kubernetes/kubectl

@k8s-bot
Copy link

k8s-bot commented Sep 1, 2015

GCE e2e build/test failed for commit a5f5e5ef2aaa2020866783d32f590a035a3d0534.

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 2, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/XS

@thockin
Copy link
Member

thockin commented Sep 2, 2015

@k8s-bot test this please

@thockin thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2015
@k8s-bot
Copy link

k8s-bot commented Sep 2, 2015

GCE e2e build/test failed for commit a5f5e5ef2aaa2020866783d32f590a035a3d0534.

@k8s-github-robot
Copy link

Continuous integration appears to have missed, closing and re-opening to trigger it

@0xmichalis
Copy link
Contributor

LGTM

deads2k
deads2k reviewed Sep 2, 2015
View reviewed changes
pkg/kubectl/resource/builder.go
@@ -32,6 +32,8 @@ import (
"k8s.io/kubernetes/pkg/util/errors"
)

var schemaFileExtensions = []string{".json", ".stdin", ".yaml", ".yml"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have expected this list to be shared with the bit creates content assist for the --filenames flag in the commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't know what bit you are referring to. Link to code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, phone won't let me paste, but its in bash_comp_utils.go where the filename flag is registered. It adds a hint for locating files with the right extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@j3ffml j3ffml force-pushed the resource-builder-err-msg branch from a5f5e5e to 1720ce8 Compare September 3, 2015 17:24
@k8s-bot
Copy link

k8s-bot commented Sep 3, 2015

GCE e2e build/test failed for commit 1720ce874af1047f2cda366124dc1690961cc47c.

@j3ffml j3ffml force-pushed the resource-builder-err-msg branch from 1720ce8 to cb65cfa Compare September 3, 2015 18:01
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 3, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-bot
Copy link

k8s-bot commented Sep 3, 2015

GCE e2e build/test failed for commit cb65cfa.

@j3ffml
Copy link
Contributor Author

j3ffml commented Sep 3, 2015

@deads2k ptal. Added .stdin to filename extension completions, since the builder recognizes it.

@ghost
Copy link

ghost commented Sep 3, 2015

e2e test failure was a flake due to #13534. Retrying to try to induce auto-merge by merge-bot.

@k8s-bot test this again please

@k8s-bot
Copy link

k8s-bot commented Sep 4, 2015

GCE e2e build/test passed for commit cb65cfa.

@piosz
Copy link
Member

piosz commented Sep 4, 2015

Removing LGTM since there were changes after it was given.

@piosz piosz removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 4, 2015
@deads2k
Copy link
Contributor

deads2k commented Sep 4, 2015

lgtm

@piosz piosz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 4, 2015
piosz added a commit that referenced this pull request Sep 4, 2015
@piosz
Merge pull request #13482 from jlowdermilk/resource-builder-err-msg
de92c9b 
Print recognized file extensions in resource builder error
@piosz piosz merged commit de92c9b into kubernetes:master Sep 4, 2015
@j3ffml j3ffml deleted the resource-builder-err-msg branch September 15, 2015 20:34
@liggitt liggitt mentioned this pull request Sep 25, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@thockin thockin

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect config filename yields confusing/unhelpful error message.
8 participants
@j3ffml @k8s-bot @k8s-github-robot @thockin @0xmichalis @piosz @deads2k @googlebot