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

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.