reviewdog / action-brakeman

Run brakeman with reviewdog 🐶
MIT License
39 stars 24 forks source link

Trying to capture exit code when `set -e` is active #31

Open dgholz opened 2 years ago

dgholz commented 2 years ago

I see in the new composite action, we try to capture the exit code from running & reporting the Brakeman findings:

brakeman [...] | reviewdog [...]

exit_code=$?
echo '::endgroup::'

exit $exit_code

https://github.com/reviewdog/action-brakeman/blob/3073c89bf52f7e1a5ab4ba2d5bfc88815311c131/script.sh#L49

I think this is so the ::endgroup:: can be printed to close the group, but because we're running with -e, the script will exit as soon as the first command fails.

The way to capture exit codes under set -e is like:

if ! cmd
then
  exit_code=$?
fi
cleanup
exit $exit_code

or using traps

dgholz commented 2 years ago

In my opinion, this isn't a bug and doesn't need a fix. It's just code that doesn't do anything and might confuse or mislead future maintainers into thinking it serves a purpose.