convert bootstrap.sh to POSIX (untested) #32
Conversation
It is not safe to rely on bash existing, let alone /bin/bash being the location of the binary. It is considered unsafe to script ls. This is why find exists. Quote all files. Convert bootstrap.sh to POSIX sh
|
Mr Reaper, Thank you for the PR. I'm trying to get someone with more knowledge about shell scripting to do the code review. Is there any chance you could submit it through the normal PR mechanism we use? Details are here: https://github.com/cockroachdb/cockroach/blob/master/CONTRIBUTING.md The key point is we use Phabricator. Not really necessary for a PR this size though, but we'd like to standardize on that channel for posterity. |
| rm .git/hooks/$(basename $f) | ||
| ln -s ../../$f .git/hooks/$(basename $f) | ||
| done && ls -al .git/hooks | grep githooks | ||
| for f in $(find githooks -mindepth 1 -maxdepth 1); do |
There was a problem hiding this comment.
I thought the $(...) construction was a bash-ism. Don't you have to use backquotes in plain shell?
|
@grimreaper ping on this. |
|
I've been busy, but have this in my 'open source TODO queue'. On 16 August 2014 08:36, Andrew Bonventre notifications@github.com wrote:
|
|
bootstrap.sh has changed so significantly lately that I'm going to close this PR. The same objections apply to the current implementation (as well as a growing body of additional shell scripts unfortunately). |
Persistent channel using rabbitmq & better logging
It is not safe to rely on bash existing, let alone /bin/bash being the location of the binary.
It is considered unsafe to script ls. This is why find exists.
Quote all files.
Convert bootstrap.sh to POSIX sh