I'd like to submit a PR, but first...

[housni]

I love this repo! There's a great set of tools here for users.

I would like to submit a few PR's but I wanted to first clarify if it's something you would use.

1. Generally, using #!/usr/bin/env bash is considered compatible so I'd like to make that change.
2. I'd like to make all the code shellcheck compatible.
3. I think it would be a good idea to set things like pipefail, nounset, errtrace, functrace, etc, in the scripts. Especially pipefail because I can see the scripts have some pipes so if commands in the pipes fail, we are not going to see those failures unless they are obvious. See: https://www.davidpashley.com/articles/writing-robust-shell-scripts/
4. I'd like to add a lot more functionality (I'm sure you won't have any objection to this :)). I can port some of the functionality in my own Bash library (https://github.com/housni/sheldon) to this repo, making sure I stick to the guidelines in the README.

Thoughts?

[brikis98]

I love this repo! There's a great set of tools here for users.

Thank you!

Generally, using #!/usr/bin/env bash is considered compatible so I'd like to make that change.

Can you provide more info/context around this?

I'd like to make all the code shellcheck compatible.

That sounds like a great idea!

I think it would be a good idea to set things like pipefail, nounset, errtrace, functrace, etc, in the scripts. Especially pipefail because I can see the scripts have some pipes so if commands in the pipes fail, we are not going to see those failures unless they are obvious. See: https://www.davidpashley.com/articles/writing-robust-shell-scripts/

This sounds great too. I wasn't familiar with a few of these, thx!

I can port some of the functionality in my own Bash library (https://github.com/housni/sheldon) to this repo, making sure I stick to the guidelines in the README.

Cool library! Contributions are very welcome 👍

What were you thinking of adding?

BTW, if you're going to be following our conventions, take note of one issue we recently found and still need to fix!

[housni]

Generally, using #!/usr/bin/env bash is considered compatible so I'd like to make that change.

Can you provide more info/context around this?

Sure. The rationale is that, bash doesn't always live in /usr/bin or /bin but env always exists at /usr/bin/env. Since env knows where bash lives, we can use it to get the correct bash interpreter for that system.

$ env | grep -i bash
SHELL=/bin/bash

References:

1. https://stackoverflow.com/questions/21612980/why-is-usr-bin-env-bash-superior-to-bin-bash
2. https://en.wikipedia.org/wiki/Shebang_(Unix)#Portability

What were you thinking of adding?

A bunch of things like string and array functions as I have implemented them here (but according to your coding standards :)): https://github.com/housni/sheldon/tree/master/lib/sheldon/util
I'll be creating a separate PR for the extra functions so you can decide on what works and what doesn't.

BTW, if you're going to be following our conventions, take note of one issue we recently found and still need to fix!

Noted! I did notice the existing syntax and realized I had not seen that before and I was wondering if it was one of those Bash-isms that I didn't know about :)

[brikis98]

The rationale is that, bash doesn't always live in /usr/bin or /bin but env always exists at /usr/bin/env. Since env knows where bash lives, we can use it to get the correct bash interpreter for that system.

Thanks for the explanation. This sounds good to me!

A bunch of things like string and array functions as I have implemented them here (but according to your coding standards :)): https://github.com/housni/sheldon/tree/master/lib/sheldon/util
I'll be creating a separate PR for the extra functions so you can decide on what works and what doesn't.

That sounds good too!

[housni]

I've almost completed PR #11 but I was wondering if things like pipefail, nounset, errtrace, functrace, etc, even belong here. I suppose I could update the Examples section README to add something like this to the existing example:

#!/usr/bin/env bash

# our traps need access to some vars to do their job properly.
set -o errtrace
set -o functrace

# exit if any command exits with a non-zero exit status.
set -e

# exit if script uses undefined variables.
set -u

# prevent masking an error in a pipeline.
#
# look at the end of the 'Use set -e' section for an excellent explanation.
# see: https://www.davidpashley.com/articles/writing-robust-shell-scripts/
set -o pipefail

# less eager word splitting - no space.
IFS=$'\n\t'

# Make debugging easier when you use `set -x`
# See: http://wiki.bash-hackers.org/scripting/debuggingtips#making_xtrace_more_useful
export PS4='+(${BASH_SOURCE}:${LINENO}): ${FUNCNAME[0]:+${FUNCNAME[0]}(): }'

Reference: https://gist.github.com/housni/d15288cdec75108ba39419450cab2bbb

That's useful info but it's also noisy so I'll let you decide.

[brikis98]

Perhaps these should go into a flags.sh that all the scripts source right at the top?

[housni]

We can have a flags.sh that every script sources. Or we can have something like a bootstrap.sh which we'd ask users to source at the top of their user scripts, if they want to.

What do you think works better here?

[brikis98]

A bootstrap.sh sounds good. Note that we'd need to update the docs to recommend sourceing this file right at the top and update the tests in this repo to do the same.

[housni]

Sounds great!

Also, I'll have CircleCI run shellcheck as a part of CI.

[housni]

I think that PR is ready to be merged in. Can we hook up CircleCI? I run shellcheck via CircleCI now.
Btw, added bootstrap.sh caused the tests to exit. There's a lot of tests so I thought it would be best if it had it's own PR in the interest of completing PR #11.

[housni]

Btw, CircleCI should run shellcheck against every file that begins with #!/usr/bin/env bash regardless of the file extension.

[housni]

Scratch that, I have one more thing to do before we can merge!

[brikis98]

Scratch that, I have one more thing to do before we can merge!

OK, let me know when it's ready for another review!