enable shellcheck and fix existed issues #980
Conversation
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
|
|
||
| # if num_rows=1m => output_files=50, scale linearly | ||
| output_num_files=$(( ( $num_rows * $num_cols + 3000 * 20000 - 1 ) / ( 3000 * 20000 ) )) | ||
| output_num_files=$(( ( num_rows * num_cols + 3000 * 20000 - 1 ) / ( 3000 * 20000 ) )) |
There was a problem hiding this comment.
fix note:
| python $gen_data_script classification \ | ||
| --n_informative $( expr $num_cols / 3 ) \ | ||
| --n_redundant $( expr $num_cols / 3 ) \ | ||
| --n_informative $(( num_cols / 3 )) \ |
There was a problem hiding this comment.
fix note: expr is antiquated. Consider rewriting this using
Signed-off-by: YanxuanLiu <[email protected]>
| MODE=${1:-all} | ||
| shift | ||
| EXTRA_ARGS=$@ | ||
| EXTRA_ARGS=("$@") |
There was a problem hiding this comment.
fix warning: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate. [SC2124]
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
| @@ -1,4 +1,5 @@ | |||
| # Copyright (c) 2024, NVIDIA CORPORATION. | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
fix error: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive. [SC2148]
| while [[ $status != "TERMINATED" ]] && [[ $status != "ERROR" ]]; do | ||
| echo -n "." | ||
| if [[ $TIME_LIMIT != "" ]] && (( $duration > $TIME_LIMIT )) | ||
| if [[ $TIME_LIMIT != "" ]] && (( duration > TIME_LIMIT )) |
There was a problem hiding this comment.
fix note:
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
Signed-off-by: YanxuanLiu <[email protected]>
|
build |
There was a problem hiding this comment.
Pull Request Overview
This PR enables shellcheck linting for shell scripts and fixes existing shellcheck violations across the codebase.
- Adds a GitHub workflow to run shellcheck on shell scripts in pull requests
- Fixes variable expansion issues by removing
$prefixes in arithmetic expressions - Adds proper shebang lines to shell scripts that were missing them
- Corrects string quoting and array handling issues
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/shell-check.yml | Adds shellcheck workflow with excluded error codes |
| python/tests/discover_gpu.sh | Fixes string quoting and updates copyright year |
| python/run_test.sh | Removes unnecessary $ prefixes in arithmetic expressions |
| python/run_benchmark.sh | Fixes array handling, arithmetic expressions, and string quoting |
| python/benchmark/databricks/*.sh | Adds missing shebang lines and fixes arithmetic expressions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
YanxuanLiu
requested review from
NvTimLiu,
eordentlich,
lijinf2,
pxLi,
rishic3 and
yinqingh
|
@eordentlich @rishic3 Could you help to review the changes? Thanks! |
The All the changes of this PR have been verified locally. |
4 participants
enable shellcheck and fix existed issues