fgerlits commented on a change in pull request #1026:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1026#discussion_r603462998
##########
File path: bstrp_functions.sh
##########
@@ -75,30 +75,31 @@ set_incompatible_with(){
}
print_multi_option_status(){
- feature="$1"
feature_status=${!1}
declare -a VAR_OPTS=()
- declare VAR_OPTS=$1_OPTIONS[@]
- VAR_OPTS=$1_OPTIONS[@]
+ declare VAR_OPTS=("$1_OPTIONS[@]")
+ VAR_OPTS=("$1_OPTIONS[@]")
for option in "${!VAR_OPTS}" ; do
if [ "${option}" = "$feature_status" ]; then
+ # shellcheck disable=SC2059
printf "${RED}"
fi
+ # shellcheck disable=SC2059
printf "${option}"
+ # shellcheck disable=SC2059
printf "${NO_COLOR} "
Review comment:
This could be rewritten as
```suggestion
if [ "${option}" = "$feature_status" ]; then
printf "%b%s%b " "${RED}" "${option}" "${NO_COLOR}"
else
printf "%s " "${option}"
fi
```
which I think would be more readable.
##########
File path: centos.sh
##########
@@ -47,20 +46,20 @@ install_bison() {
if [ "$OS_MAJOR" = "6" ]; then
BISON_INSTALLED="false"
if [ -x "$(command -v bison)" ]; then
- BISON_VERSION=`bison --version | head -n 1 | awk '{print $4}'`
- BISON_MAJOR=`echo $BISON_VERSION | cut -d. -f1`
+ BISON_VERSION=$(bison --version | head -n 1 | awk '{print $4}')
+ BISON_MAJOR=$(echo "$BISON_VERSION" | cut -d. -f1)
if (( BISON_MAJOR >= 3 )); then
BISON_INSTALLED="true"
fi
fi
if [ "$BISON_INSTALLED" = "false" ]; then
wget https://ftp.gnu.org/gnu/bison/bison-3.0.4.tar.xz
tar xvf bison-3.0.4.tar.xz
- pushd bison-3.0.4
+ pushd bison-3.0.4 || return
Review comment:
Elsewhere you used `exit 1`, which I think is better. If we silently
return, we are likely to get a cryptic error message later.
##########
File path: .github/workflows/ci.yml
##########
@@ -159,7 +159,7 @@ jobs:
sudo apt install -y ccache libfl-dev libpcap-dev libboost-all-dev
echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
- id: build
- run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS=
-DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQLITE=ON
-DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4
VERBOSE=1 && make test ARGS="--timeout 300 -j2 --output-on-failure"
+ run: ./bootstrap.sh -e -t && cd build && cmake -DUSE_SHARED_LIBS=
-DCMAKE_BUILD_TYPE=Release -DENABLE_BUSTACHE=ON -DENABLE_SQLITE=ON
-DENABLE_PCAP=ON -DSTRICT_GSL_CHECKS=AUDIT -DFAIL_ON_WARNINGS=ON .. && make -j4
VERBOSE=1 && make test ARGS="--timeout 300 -j2 --output-on-failure" && make
shellcheck
Review comment:
I would make this a separate `step` rather than adding it to the end of
the `build` step. That would make it easier to find the shellcheck output in
the Checks window.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
