[libvirt PATCH 2/4] ci: Rewrite list-images from Bash to Python

Andrea Bolognani abologna at redhat.com
Tue Feb 16 13:40:32 UTC 2021


On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote:
> +# skip the "ci-" prefix each of our container images' name has
> +names = [i["name"][3:] for i in data]

I would prefer something like

  PREFIX = "ci-"
  names = [i["name"][len(PREFIX):] for i in data]

here, rather than hardcoding the length of the string.

> +names_native = [name for name in names if "cross" not in name]
> +names_cross = [name for name in names if "cross" in name]
> +
> +print("Available x86 container images:\n")
> +print("\t" + "\n\t".join(names_native))

Please use two or four spaces instead of tabs.

More high-level feedback about the patch:

  * you should drop the sh implementation at the same time as you're
    adding the python one;

  * adding this standalone script in one patch only to refactor most
    of its code away to a separate module in the next one leads to
    unnecessary churn and more work for the reviewer: please just add
    the script in its intended form in the first place.

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list