From 16dd8b3ed94d5cd217d22a26c18dca52bfca115e Mon Sep 17 00:00:00 2001 From: Sean Dague Date: Mon, 3 Feb 2014 09:10:54 +0900 Subject: [PATCH] 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 --- lib/baremetal | 3 +-- lib/heat | 3 +-- lib/neutron_plugins/bigswitch_floodlight | 6 ++--- lib/neutron_plugins/nec | 3 +-- lib/neutron_thirdparty/bigswitch_floodlight | 3 +-- stack.sh | 4 +-- tests/functions.sh | 12 +++------ tools/bash8.py | 29 +++++++++++++++++++++ tools/xen/install_os_domU.sh | 3 +-- tools/xen/scripts/install-os-vpx.sh | 9 +++---- tools/xen/scripts/on_exit.sh | 6 ++--- tools/xen/test_functions.sh | 6 ++--- 12 files changed, 49 insertions(+), 38 deletions(-) diff --git a/lib/baremetal b/lib/baremetal index a0df85e700..d8cd7e936c 100644 --- a/lib/baremetal +++ b/lib/baremetal @@ -431,8 +431,7 @@ function upload_baremetal_image() { 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 f171cb450c..9f5dd8b588 100644 --- a/lib/heat +++ b/lib/heat @@ -186,8 +186,7 @@ function disk_image_create { 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 93ec497bb9..1e4aa00121 100644 --- a/lib/neutron_plugins/bigswitch_floodlight +++ b/lib/neutron_plugins/bigswitch_floodlight @@ -44,16 +44,14 @@ function neutron_plugin_configure_plugin_agent() { 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 d8d8b7ce7e..1cb2fef533 100644 --- a/lib/neutron_plugins/nec +++ b/lib/neutron_plugins/nec @@ -106,8 +106,7 @@ function _neutron_setup_ovs_tunnels() { 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 1fd4fd801a..24c10443b7 100644 --- a/lib/neutron_thirdparty/bigswitch_floodlight +++ b/lib/neutron_thirdparty/bigswitch_floodlight @@ -24,8 +24,7 @@ function init_bigswitch_floodlight() { 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 45d47c819c..15e14303cf 100755 --- a/stack.sh +++ b/stack.sh @@ -1124,8 +1124,8 @@ fi # 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 95dafe1028..06a4134abf 100755 --- a/tests/functions.sh +++ b/tests/functions.sh @@ -49,8 +49,7 @@ function test_enable_service() { 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 @@ function test_disable_service() { 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 @@ echo "Testing disable_all_services()" 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 @@ function test_disable_negated_services() { 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 2623358182..9fb51ecc9e 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 @@ def print_error(error, line): 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 @@ def check_files(files): 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 41b184c6ac..d172c7ba1b 100755 --- a/tools/xen/install_os_domU.sh +++ b/tools/xen/install_os_domU.sh @@ -194,8 +194,7 @@ function wait_for_VM_to_halt() { 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 7b0d891493..8412fdc3ca 100755 --- a/tools/xen/scripts/install-os-vpx.sh +++ b/tools/xen/scripts/install-os-vpx.sh @@ -63,8 +63,7 @@ get_params() ;; esac done - if [[ -z $BRIDGE ]] - then + if [[ -z $BRIDGE ]]; then BRIDGE=xenbr0 fi @@ -91,8 +90,7 @@ xe_min() 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 @@ destroy_vifs() { 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 a4db39c225..2441e3d84a 100755 --- a/tools/xen/scripts/on_exit.sh +++ b/tools/xen/scripts/on_exit.sh @@ -7,8 +7,7 @@ declare -a on_exit_hooks 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 @@ add_on_exit() { 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 373d996760..838f86a525 100755 --- a/tools/xen/test_functions.sh +++ b/tools/xen/test_functions.sh @@ -227,16 +227,14 @@ function test_get_local_sr_path { } [ "$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