Re: [libvirt] [libvirt-jenkins-ci PATCH 2/5] ansible: Introduce the 'manage' tool
On Tue, 2017-10-17 at 15:21 +0200, Pavel Hrdina wrote:
> +do_list() {
> + INVENTORY=$(grep "^inventory\\s*=" ansible.cfg 2>/dev/null | sed
"s/^.*=\\s*//g")
> + INVENTORY=${INVENTORY:-/etc/ansible/hosts}
I don't think that there is a need to include system-wide inventory
since we have our own inventory and we don't use the system-wide
inventory at all.
Good point. I turned the absence of the local inventory file into
an error, and...
> + grep -v '^\[' "$INVENTORY" | sort -u
... improved the regular expression so that empty lines and comments
will be ignored in addition to group names.
> +case "$1" in
> + list) do_list ;;
> + prepare|update) do_prepare "$2" ;;
> + *help) do_help ;;
> + *) die "Usage: $PROGRAM_NAME ACTION [OPTIONS]" ;;
How about grouping *help) and *) together? I was pretending to be a
basic user and I've run this script without any option at all and this
was the only output. So it would be nice to print the help itself
or mention that there is some "--help" option.
I went one step further and dropped the command-specific error
message in favor of displaying the help text. One less string, and
it's actually more informative :)
--
Andrea Bolognani / Red Hat / Virtualization