[libvirt] [libvirt-jenkins-ci PATCH 2/5] ansible: Introduce the 'manage' tool

Andrea Bolognani abologna at redhat.com
Tue Oct 17 14:02:33 UTC 2017


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




More information about the libvir-list mailing list