chore: Fix warnings produced by shellcheck on bench.sh #12303
Conversation
edmondop
left a comment
edmondop
left a comment
There was a problem hiding this comment.
I wonder if we want to add something here https://github.com/apache/datafusion/tree/main/dev (lint_scripts.sh) maybe? And maybe also a task in CI?
| usage | ||
| ;; | ||
| -*|--*) | ||
| -*) |
There was a problem hiding this comment.
The removed pattern is a subset of -* and will therefore never match. https://www.shellcheck.net/wiki/SC2221
Agreed. If people agree that this is a change in the desired direction. Then it makes sense to address the same issues in other bash scripts in this repo and add a CI check as a follow up. |
| echo "RESULTS_DIR: ${RESULTS_DIR}" | ||
| echo "CARGO_COMMAND: ${CARGO_COMMAND}" | ||
| echo "PREFER_HASH_JOIN": ${PREFER_HASH_JOIN} | ||
| echo "PREFER_HASH_JOIN: ${PREFER_HASH_JOIN}" |
comphead
left a comment
comphead
left a comment
There was a problem hiding this comment.
Thanks @eejbyfeldt I feel this PR is great.
This is a nice idea. Not sure about having another CI task though. |
3 participants
Which issue does this PR close?
N/A
Rationale for this change
Many of the warnings produced by shellcheck are indications of bugs in the script that would break under certain circumstances.
What changes are included in this PR?
The following warnings was previously produced:
SC2006
SC2035
SC2045
SC2086
SC2140
SC2221 (SC2222)
Are these changes tested?
Manually running the script.
Are there any user-facing changes?
No.