refactor(shellcheck): fix all remaining warnings/errors #1019
Conversation
gforsyth
added
improvement
|
@jameslamb one more for you here |
build.sh
Outdated
| if [[ ${CMAKE_TARGET} != "" ]]; then | ||
| echo "-- Compiling targets: ${CMAKE_TARGET}, verbose=${VERBOSE_FLAG}" | ||
| if [[ ${CMAKE_TARGET[*]} != "" ]]; then | ||
| echo "-- Compiling targets: ${CMAKE_TARGET[*]}, verbose=${VERBOSE_FLAG}" |
There was a problem hiding this comment.
| echo "-- Compiling targets: ${CMAKE_TARGET[*]}, verbose=${VERBOSE_FLAG}" | |
| echo "-- Compiling targets: ${CMAKE_TARGET[@]}, verbose=${VERBOSE_FLAG}" |
Similar to my comment above... I think here you want the values, not the keys (because there are no keys).
There was a problem hiding this comment.
Yeah, I think this is another weird implicit vs. explicit array concatenation thing -- shellcheck doesn't like it when strings and arrays are mixed: https://github.com/koalaman/shellcheck/wiki/SC2145
There was a problem hiding this comment.
huh, well, I'm definitely wrong here because that didn't output any values
There was a problem hiding this comment.
Hmm, or maybe CMAKE_TARGET was just empty? I've pushed up a different way of spelling this, in any case, but I think this should work.
> CMAKE_TARGET=('one' 'two')
> echo "-- Compiling targets: ${CMAKE_TARGET[*]}"
-- Compiling targets: one twoCo-authored-by: James Lamb <[email protected]>
Co-authored-by: James Lamb <[email protected]>
There was a problem hiding this comment.
Would it make sense (if it's possible) to exclude this file from cpp-ownership?
Finishes up the shellcheck work tracked in rapidsai/build-planning#135