Skip to content

add fallbacks in checks with '-z' - #179

Merged
jameslamb merged 3 commits into
mainfrom
fix/arg-processing
May 14, 2025
Merged

add fallbacks in checks with '-z'#179
jameslamb merged 3 commits into
mainfrom
fix/arg-processing

Conversation

@jameslamb

Copy link
Copy Markdown
Member

I've been spending a lot of time in this repo recently and have seen a recurring pattern.

Checks like this:

if [ -z "$1" ]; then
  echo "you have to pass an argument"
  exit 1
fi

If code like that is run when set -u is set, that informative error message and other code inside the then block will never be executed... bash will exit immediately with an error like "unbound variable ...".

This adds a fallback to null in all such checks I could find, to avoid that.

Notes for Reviewers

How I found these

git grep -E '\-n'
git grep -E '\-z'

Recent Examples

@jameslamb
jameslamb requested a review from a team as a code owner May 14, 2025 00:07
@jameslamb
jameslamb requested review from gforsyth and removed request for a team May 14, 2025 00:07
@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels May 14, 2025

@gforsyth gforsyth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Big fan of this -- just one question on a stray commented out line

Comment thread tools/rapids-upload-to-s3 Outdated
@jameslamb
jameslamb requested a review from gforsyth May 14, 2025 14:27

@gforsyth gforsyth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@jameslamb
jameslamb merged commit 0375464 into main May 14, 2025
@jameslamb
jameslamb deleted the fix/arg-processing branch June 2, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants