Skip to content

Conversation

marun
Copy link
Contributor

@marun marun commented Aug 16, 2023

Why this should be merged

Ensures all scripts set the following best-practice bash options:

set -o errexit # Ensure script failure on error. Without it execution continues in most cases (excepting some explicit conditional checks).
set -o nounset # Ensures an error when an unset variable is referenced (i.e. requires explicit variable initialization).
set -o pipefail # Ensures an error when a pipe call (both with `|` and `$( )`) fails.

For simplicity the set -euo pipefail one-liner is used to ensure that any future changes to the standard settings can target a single line.

@marun marun requested a review from StephenButtolph as a code owner August 16, 2023 17:14
Copy link
Contributor

@joshua-kim joshua-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these changes look good, but would it make sense for us to just source a common header script that ran all these common flags instead?

@marun
Copy link
Contributor Author

marun commented Aug 16, 2023

I think these changes look good, but would it make sense for us to just source a common header script that ran all these common flags instead?

Good call, I'll update accordingly.

@marun
Copy link
Contributor Author

marun commented Aug 16, 2023

I switched to set -euo pipefail which is a one-liner that does the same thing as the previous 3 lines. Should simplify maintenance and is arguably simpler than ensuring that all scripts import a common file (due to scripts being potentially being run with different working directories.

@marun
Copy link
Contributor Author

marun commented Aug 16, 2023

Have to fix the upgrade test script, it isn't yet compatible. Done

@marun marun force-pushed the cleanup-bash-scripts branch from 546cb2e to 70a08fc Compare August 16, 2023 20:49
@StephenButtolph StephenButtolph added the cleanup Code quality improvement label Aug 17, 2023
@StephenButtolph StephenButtolph added this to the v1.10.9 milestone Aug 17, 2023
@StephenButtolph StephenButtolph merged commit 6c8a7d4 into dev Aug 17, 2023
@StephenButtolph StephenButtolph deleted the cleanup-bash-scripts branch August 17, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants