introduce if/then & for/do rules
we mostly have a consistent style on if/then & for/do in devstack,
except when we don't. This attempts to build a set of rules to
enforce this.
Because there are times when lines are legitimately long, and there
is a continuation, this starts off ignoring if and for loops with
continuations. But for short versions, we should enforce this.
Changes to make devstack pass are included. The fact that the
cleanup patch was so small is pretty solid reason that this is
actually the style we've all agreed to.
Part of a git stash from hong kong that I finally cleaned up.
Change-Id: I6376d7afd59cc5ebba9ed69e5ee784a3d5934a10
diff --git a/lib/baremetal b/lib/baremetal
index a0df85e..d8cd7e9 100644
--- a/lib/baremetal
+++ b/lib/baremetal
@@ -431,8 +431,7 @@
function clear_baremetal_of_all_nodes() {
list=$(nova baremetal-node-list | awk -F '| ' 'NR>3 {print $2}' )
- for node in $list
- do
+ for node in $list; do
nova baremetal-node-delete $node
done
}
diff --git a/lib/heat b/lib/heat
index f171cb4..9f5dd8b 100644
--- a/lib/heat
+++ b/lib/heat
@@ -186,8 +186,7 @@
local elements=$2
local arch=$3
local output=$TOP_DIR/files/$4
- if [[ -f "$output.qcow2" ]];
- then
+ if [[ -f "$output.qcow2" ]]; then
echo "Image file already exists: $output_file"
else
ELEMENTS_PATH=$elements_path disk-image-create \
diff --git a/lib/neutron_plugins/bigswitch_floodlight b/lib/neutron_plugins/bigswitch_floodlight
index 93ec497..1e4aa00 100644
--- a/lib/neutron_plugins/bigswitch_floodlight
+++ b/lib/neutron_plugins/bigswitch_floodlight
@@ -44,16 +44,14 @@
function neutron_plugin_configure_service() {
iniset /$Q_PLUGIN_CONF_FILE restproxy servers $BS_FL_CONTROLLERS_PORT
iniset /$Q_PLUGIN_CONF_FILE restproxy servertimeout $BS_FL_CONTROLLER_TIMEOUT
- if [ "$BS_FL_VIF_DRIVER" = "ivs" ]
- then
+ if [ "$BS_FL_VIF_DRIVER" = "ivs" ]; then
iniset /$Q_PLUGIN_CONF_FILE nova vif_type ivs
fi
}
function neutron_plugin_setup_interface_driver() {
local conf_file=$1
- if [ "$BS_FL_VIF_DRIVER" = "ivs" ]
- then
+ if [ "$BS_FL_VIF_DRIVER" = "ivs" ]; then
iniset $conf_file DEFAULT interface_driver neutron.agent.linux.interface.IVSInterfaceDriver
else
iniset $conf_file DEFAULT interface_driver neutron.agent.linux.interface.OVSInterfaceDriver
diff --git a/lib/neutron_plugins/nec b/lib/neutron_plugins/nec
index d8d8b7c..1cb2fef 100644
--- a/lib/neutron_plugins/nec
+++ b/lib/neutron_plugins/nec
@@ -106,8 +106,7 @@
local id=0
GRE_LOCAL_IP=${GRE_LOCAL_IP:-$HOST_IP}
if [ -n "$GRE_REMOTE_IPS" ]; then
- for ip in ${GRE_REMOTE_IPS//:/ }
- do
+ for ip in ${GRE_REMOTE_IPS//:/ }; do
if [[ "$ip" == "$GRE_LOCAL_IP" ]]; then
continue
fi
diff --git a/lib/neutron_thirdparty/bigswitch_floodlight b/lib/neutron_thirdparty/bigswitch_floodlight
index 1fd4fd8..24c1044 100644
--- a/lib/neutron_thirdparty/bigswitch_floodlight
+++ b/lib/neutron_thirdparty/bigswitch_floodlight
@@ -24,8 +24,7 @@
sudo ovs-vsctl --no-wait br-set-external-id ${OVS_BRIDGE} bridge-id ${OVS_BRIDGE}
ctrls=
- for ctrl in `echo ${BS_FL_CONTROLLERS_PORT} | tr ',' ' '`
- do
+ for ctrl in `echo ${BS_FL_CONTROLLERS_PORT} | tr ',' ' '`; do
ctrl=${ctrl%:*}
ctrls="${ctrls} tcp:${ctrl}:${BS_FL_OF_PORT}"
done
diff --git a/stack.sh b/stack.sh
index 45d47c8..15e1430 100755
--- a/stack.sh
+++ b/stack.sh
@@ -1124,8 +1124,8 @@
# Create a randomized default value for the keymgr's fixed_key
if is_service_enabled nova; then
FIXED_KEY=""
- for i in $(seq 1 64);
- do FIXED_KEY+=$(echo "obase=16; $(($RANDOM % 16))" | bc);
+ for i in $(seq 1 64); do
+ FIXED_KEY+=$(echo "obase=16; $(($RANDOM % 16))" | bc);
done;
iniset $NOVA_CONF keymgr fixed_key "$FIXED_KEY"
fi
diff --git a/tests/functions.sh b/tests/functions.sh
index 95dafe1..06a4134 100755
--- a/tests/functions.sh
+++ b/tests/functions.sh
@@ -49,8 +49,7 @@
ENABLED_SERVICES="$start"
enable_service $add
- if [ "$ENABLED_SERVICES" = "$finish" ]
- then
+ if [ "$ENABLED_SERVICES" = "$finish" ]; then
echo "OK: $start + $add -> $ENABLED_SERVICES"
else
echo "changing $start to $finish with $add failed: $ENABLED_SERVICES"
@@ -76,8 +75,7 @@
ENABLED_SERVICES="$start"
disable_service "$del"
- if [ "$ENABLED_SERVICES" = "$finish" ]
- then
+ if [ "$ENABLED_SERVICES" = "$finish" ]; then
echo "OK: $start - $del -> $ENABLED_SERVICES"
else
echo "changing $start to $finish with $del failed: $ENABLED_SERVICES"
@@ -102,8 +100,7 @@
ENABLED_SERVICES=a,b,c
disable_all_services
-if [[ -z "$ENABLED_SERVICES" ]]
-then
+if [[ -z "$ENABLED_SERVICES" ]]; then
echo "OK"
else
echo "disabling all services FAILED: $ENABLED_SERVICES"
@@ -118,8 +115,7 @@
ENABLED_SERVICES="$start"
disable_negated_services
- if [ "$ENABLED_SERVICES" = "$finish" ]
- then
+ if [ "$ENABLED_SERVICES" = "$finish" ]; then
echo "OK: $start + $add -> $ENABLED_SERVICES"
else
echo "changing $start to $finish failed: $ENABLED_SERVICES"
diff --git a/tools/bash8.py b/tools/bash8.py
index 2623358..9fb51ec 100755
--- a/tools/bash8.py
+++ b/tools/bash8.py
@@ -21,9 +21,19 @@
# Currently Supported checks
#
# Errors
+# Basic white space errors, for consistent indenting
# - E001: check that lines do not end with trailing whitespace
# - E002: ensure that indents are only spaces, and not hard tabs
# - E003: ensure all indents are a multiple of 4 spaces
+#
+# Structure errors
+#
+# A set of rules that help keep things consistent in control blocks.
+# These are ignored on long lines that have a continuation, because
+# unrolling that is kind of "interesting"
+#
+# - E010: *do* not on the same line as *for*
+# - E011: *then* not on the same line as *if*
import argparse
import fileinput
@@ -51,6 +61,23 @@
print(" - %s: L%s" % (fileinput.filename(), fileinput.filelineno()))
+def not_continuation(line):
+ return not re.search('\\\\$', line)
+
+def check_for_do(line):
+ if not_continuation(line):
+ if re.search('^\s*for ', line):
+ if not re.search(';\s*do(\b|$)', line):
+ print_error('E010: Do not on same line as for', line)
+
+
+def check_if_then(line):
+ if not_continuation(line):
+ if re.search('^\s*if \[', line):
+ if not re.search(';\s*then(\b|$)', line):
+ print_error('E011: Then non on same line as if', line)
+
+
def check_no_trailing_whitespace(line):
if re.search('[ \t]+$', line):
print_error('E001: Trailing Whitespace', line)
@@ -100,6 +127,8 @@
check_no_trailing_whitespace(logical_line)
check_indents(logical_line)
+ check_for_do(logical_line)
+ check_if_then(logical_line)
def get_options():
diff --git a/tools/xen/install_os_domU.sh b/tools/xen/install_os_domU.sh
index 41b184c..d172c7b 100755
--- a/tools/xen/install_os_domU.sh
+++ b/tools/xen/install_os_domU.sh
@@ -194,8 +194,7 @@
while true
do
state=$(xe_min vm-list name-label="$GUEST_NAME" power-state=halted)
- if [ -n "$state" ]
- then
+ if [ -n "$state" ]; then
break
else
echo -n "."
diff --git a/tools/xen/scripts/install-os-vpx.sh b/tools/xen/scripts/install-os-vpx.sh
index 7b0d891..8412fdc 100755
--- a/tools/xen/scripts/install-os-vpx.sh
+++ b/tools/xen/scripts/install-os-vpx.sh
@@ -63,8 +63,7 @@
;;
esac
done
- if [[ -z $BRIDGE ]]
- then
+ if [[ -z $BRIDGE ]]; then
BRIDGE=xenbr0
fi
@@ -91,8 +90,7 @@
find_network()
{
result=$(xe_min network-list bridge="$1")
- if [ "$result" = "" ]
- then
+ if [ "$result" = "" ]; then
result=$(xe_min network-list name-label="$1")
fi
echo "$result"
@@ -121,8 +119,7 @@
{
local v="$1"
IFS=,
- for vif in $(xe_min vif-list vm-uuid="$v")
- do
+ for vif in $(xe_min vif-list vm-uuid="$v"); do
xe vif-destroy uuid="$vif"
done
unset IFS
diff --git a/tools/xen/scripts/on_exit.sh b/tools/xen/scripts/on_exit.sh
index a4db39c..2441e3d 100755
--- a/tools/xen/scripts/on_exit.sh
+++ b/tools/xen/scripts/on_exit.sh
@@ -7,8 +7,7 @@
on_exit()
{
- for i in $(seq $((${#on_exit_hooks[*]} - 1)) -1 0)
- do
+ for i in $(seq $((${#on_exit_hooks[*]} - 1)) -1 0); do
eval "${on_exit_hooks[$i]}"
done
}
@@ -17,8 +16,7 @@
{
local n=${#on_exit_hooks[*]}
on_exit_hooks[$n]="$*"
- if [[ $n -eq 0 ]]
- then
+ if [[ $n -eq 0 ]]; then
trap on_exit EXIT
fi
}
diff --git a/tools/xen/test_functions.sh b/tools/xen/test_functions.sh
index 373d996..838f86a 100755
--- a/tools/xen/test_functions.sh
+++ b/tools/xen/test_functions.sh
@@ -227,16 +227,14 @@
}
[ "$1" = "run_tests" ] && {
- for testname in $($0)
- do
+ for testname in $($0); do
echo "$testname"
before_each_test
(
set -eux
$testname
)
- if [ "$?" != "0" ]
- then
+ if [ "$?" != "0" ]; then
echo "FAIL"
exit 1
else