Hacker News new | ask | show | jobs
by nickjj 1403 days ago
Thanks.

My thought process around using braces when they're not needed is mainly around consistency. If you pick and choose when to add them then you need to make a decision every time you add a variable. I've written about that here: https://nickjanetakis.com/blog/why-you-should-put-braces-aro...

That is a good call about `set -u`, it's something I've been using more recently but I haven't added it into that script yet but thanks for the reminder, I will soon. I ended up making a post about that here: https://nickjanetakis.com/blog/prevent-unset-variables-in-yo...

Another small thing I've been doing recently is defining options like this:

    set -o errexit
    set -o pipefail
    set -o nounset
It's a little more explicit on what each option does. It might be just enough context to avoid having to look up what something does. Philosophy wise that's also something I've been doing semi-recently which is to use long form flags over short flags in scripts https://nickjanetakis.com/blog/when-to-use-long-word-or-shor....
1 comments

It adds too much visual noise for me, especially since you already need to double-quote the variables to protect against whitespace expansion. The rules around when braces are needed are simple so I leave them off when they aren't necessary. The rules around when double-quotes are needed are much more subtle, so I almost always use double-quotes, even when they aren't needed. e.g.

   foo=$bar  # quoting not needed but I'd still do this:
   foo="$bar"
A bug-bear of mine is unquoted variables especially with braces, even when using them for optional args:

   ${TTY}
Using your original script as an example, I'd prefer this:

    dc_exec=(docker-compose "${DC:-exec}")
    dc_run=(docker-compose run)
    if [[ ! -t ]]; then
      # we have no TTY so we're probably running under CI
      # which means we need to disable TTY docker allocation
      dc_exec=("${dc_exec[@]}" --no-TTY)
      dc_run=("${dc_run[@]}" --no-TTY)
    fi

    _dc() {
      "${dc_exec[@]}" "$@"
    }

    _build_run_down() {
      docker-compose build
      "${dc_run[@]}" "$@"
      docker-compose down
    }
Of course, this uses bash arrays and isn't POSIX. But the ergonomics of using an array for construction a long command are so much nicer than backslashes or long lines that I use them all the time now.

   cmd=(
      /usr/bin/some-command
      --option1="foo"
      --option2="bar"
      --option3="baz"
   )
   "${cmd[@]}"

I also prefer using self-documenting long-options like above when writing shell scripts.

Another thing that goes along with set -u is to fail early. So for example, your script seems to require global POSTGRES_USER, etc, so why not:

   set -o nounset
   # fail early on required variables
   : "${POSTGRES_USER}"
   : "${POSTGRES_PASSWORD}"
Happy to code-golf shell scripts. There's usually nothing but hostility toward them around here. :-)
Thanks.

The postgres variables are sourced in the line above I use them, they will always be set. It's a hard requirement of the Docker postgres image. I did end up pushing up the nounset change and it didn't complain about it since it's set from being sourced.

Doh, I missed that.

Aside, but I gotta say, lots of good stuff here:

https://nickjanetakis.com/blog/

I'm not personally a fan of videos, but I have plenty of collegeues who are that I'm going to happily start pointing at your videos. Some of these will be very handy references I can add to code reviews.

Thanks, I really appreciate it. Word of mouth helps a lot.