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