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

Erik Skultety eskultet at redhat.com
Tue Feb 16 14:44:46 UTC 2021


On Tue, Feb 16, 2021 at 02:40:32PM +0100, Andrea Bolognani wrote:
> 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.

Sure, will do.

> 
> 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.

As you wish, I approached the way I'd appreciate it - introduce a drop 1:1
replacement and then change individual bits later...oh well, all of us have
different taste.

Thanks,
Erik




More information about the libvir-list mailing list