Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure consistent use of best-practice set -o in all scripts #1864

Merged
merged 5 commits into from
Aug 17, 2023

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.

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

@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
41 checks passed
@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.

5 participants