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

Show help message in kubectl.sh if no args. #4393

Merged
merged 1 commit into from
Feb 12, 2015

Conversation

erictune
Copy link
Member

When no args are passed to a script, "$@" is unset,
which causes a shell error in "nounset" mode.

This change passes an empty string to kubectl in that case
so it can print help.

This also updates docs/kubectl.md because the hook told me to.

@erictune
Copy link
Member Author

didn't need docs change. needed make clean before commit hook.

@erictune
Copy link
Member Author

assigned to @filbranden for advice on whether this is the best practice in bash.

@erictune erictune changed the title Handle unset dollar-at. Show help message in kubectl.sh if no args. Feb 12, 2015
@filbranden
Copy link
Contributor

Problem with "${@-}" is that it expands to a single empty argument when $@ is empty.

Better is either one of ${1+"$@"} (which is really common shell slang) or "${@+$@}" or similar.

(Note that "$@" trigerring nounset when empty only seems to happen on the bash version shipped in Mac OS, I don't think this is causing problems with the Linux version.)

@roberthbailey
Copy link
Contributor

Dupe of #4390?

When no args are passed to a script, "$@" is unset,
which causes a shell error in "nounset" mode.

This change passes an empty string to kubectl in that case
so it can print help.
@erictune
Copy link
Member Author

PTAL

@erictune
Copy link
Member Author

yes dup.

@erictune
Copy link
Member Author

but this one is further along in review so drop that one.

filbranden added a commit that referenced this pull request Feb 12, 2015
Show help message in kubectl.sh if no args.
@filbranden filbranden merged commit e54fe11 into kubernetes:master Feb 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants