[ci] fix some shellcheck errors in scripts #6504
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…icrosoft#6498) - Errors- SC2086 (info) : Double quote to prevent globbing and word splitting
…t#6498) - Fixed SC2086 (info): Double quote to prevent globbing and word splitting.
…icrosoft#6498) - fixed SC2086 (info): Double quote to prevent globbing and word splitting.
shreyash2002
requested review from
guolinke,
jameslamb,
shiyu1994,
jmoralez and
borchero
as code owners
|
@microsoft-github-policy-service agree |
shreyash2002
changed the title
|
hi @jameslamb |
jameslamb
changed the title
jameslamb
requested changes
Jun 24, 2024
There was a problem hiding this comment.
Thanks for taking the time to contribute to LightGBM!
I've requested some changes here. I've also made some edits:
- removed
fixed #6498from the PR title... this does not FIX that issue, it just helps make some progress towards it - added a link to [ci] enforce 'shellcheck' checks #6498 in the PR description... please always do this when you contribute here and your contribution relates to other discussions in the project
- Used double quotes around entire word as suggested - microsoft#6498
jameslamb
requested changes
Jun 28, 2024
Comment on lines
+42
to
+50
| "llvm-${CLANG_VERSION}"-dev \ | ||
| "llvm-${CLANG_VERSION}"-tools \ | ||
| "libomp-${CLANG_VERSION}"-dev \ | ||
| "libc++-${CLANG_VERSION}"-dev \ | ||
| "libc++abi-${CLANG_VERSION}"-dev \ | ||
| "libclang-common-${CLANG_VERSION}"-dev \ | ||
| "libclang-${CLANG_VERSION}"-dev \ | ||
| "libclang-cpp${CLANG_VERSION}"-dev \ | ||
| "libunwind-${CLANG_VERSION}"-dev |
There was a problem hiding this comment.
Suggested change
| "llvm-${CLANG_VERSION}"-dev \ | |
| "llvm-${CLANG_VERSION}"-tools \ | |
| "libomp-${CLANG_VERSION}"-dev \ | |
| "libc++-${CLANG_VERSION}"-dev \ | |
| "libc++abi-${CLANG_VERSION}"-dev \ | |
| "libclang-common-${CLANG_VERSION}"-dev \ | |
| "libclang-${CLANG_VERSION}"-dev \ | |
| "libclang-cpp${CLANG_VERSION}"-dev \ | |
| "libunwind-${CLANG_VERSION}"-dev | |
| "llvm-${CLANG_VERSION}-dev" \ | |
| "llvm-${CLANG_VERSION}-tools" \ | |
| "libomp-${CLANG_VERSION}-dev" \ | |
| "libc++-${CLANG_VERSION}-dev" \ | |
| "libc++abi-${CLANG_VERSION}-dev" \ | |
| "libclang-common-${CLANG_VERSION}-dev" \ | |
| "libclang-${CLANG_VERSION}-dev" \ | |
| "libclang-cpp${CLANG_VERSION}-dev" \ | |
| "libunwind-${CLANG_VERSION}-dev" |
Please don't leave quotes in the middle of these words, as I asked in #6504 (comment).
|
|
||
| # overwriting the stuff in /usr/bin is simpler and more reliable than | ||
| # updating PATH, LD_LIBRARY_PATH, etc. | ||
| cp --remove-destination /usr/lib/llvm-${CLANG_VERSION}/bin/* /usr/bin/ | ||
| cp --remove-destination /usr/lib/llvm-"${CLANG_VERSION}"/bin/* /usr/bin/ |
There was a problem hiding this comment.
Suggested change
| cp --remove-destination /usr/lib/llvm-"${CLANG_VERSION}"/bin/* /usr/bin/ | |
| cp --remove-destination "$(echo /usr/lib/llvm-${CLANG_VERSION}/bin/*)" /usr/bin/ |
| --arg pr_branch "$(echo $pr | jq '.head.ref')" \ | ||
| --arg pr_number "$(echo "$pr" | jq '.number')" \ | ||
| --arg pr_sha "$(echo "$pr" | jq '.head.sha')" \ | ||
| --arg pr_branch "$(echo "$pr" | jq '.head.ref')" \ |
There was a problem hiding this comment.
Putting double quotes inside other double quotes like this requires escaping. To avoid that, please revert these changes and instead add the following comment on line 37 (directly above data=$():
# shellcheck disable=SC0286
|
@shreyash2002 are you planning to return to this pull request? Do you need any help? |
|
@shreyash2002 Thanks for your work here! Please pick up any other issue to contribute if you wish. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributes to #6498