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

Add shellcheck to pre-commit and fix warnings #814

Merged
merged 3 commits into from
Jan 29, 2025

Conversation

gforsyth
Copy link
Contributor

Description

shellcheck is a fast, static analysis tool for shell scripts. It's good at
flagging up unused variables, unintentional glob expansions, and other potential
execution and security headaches that arise from the wonders of bash (and
other shlangs).

This PR adds a pre-commit hook to run shellcheck on all of the sh-lang
files in the ci/ directory, and the changes requested by shellcheck to make
the existing files pass the check.

xref: rapidsai/build-planning#135

@gforsyth gforsyth requested a review from a team as a code owner January 22, 2025 17:57
@gforsyth gforsyth requested a review from raydouglass January 22, 2025 17:57
@gforsyth gforsyth added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jan 22, 2025
Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I'm good with this! Just double-check that update-version.sh change if you haven't already, I think that sed_runner thing can be really sensitive to quoting.

You can test like this, from the root of the repo:

# get all the tags (assumed you have rapidsai/cucim as the remote)
git fetch upstream --tags

# update versions
ci/release/update-version.sh '25.04.00'

If that worked, you should see most "25.02" changed to "25.04" and all "25.2" changed to "25.4".

@jameslamb jameslamb removed the request for review from raydouglass January 28, 2025 23:33
@gforsyth
Copy link
Contributor Author

If that worked, you should see most "25.02" changed to "25.04" and all "25.2" changed to "25.4".

Yep, that was definitely one of the gnarlier quoting conversions, but yes, it works!

@jameslamb
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 2a20c86 into rapidsai:branch-25.02 Jan 29, 2025
45 checks passed
@gforsyth gforsyth deleted the shellcheck branch January 29, 2025 15:20
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