Re: [Xen-devel] [PATCH v5 14/17] osstest: change the meaning of need_build_host

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[PATCH v5 14/17] osstest: change the meaning of 
need_build_host"):
> Make need_build_host store a string instead of a boolean. This is
> later going to be expanded to handle the FreeBSD build jobs.

This is all fine, but I have two style comments:

> +    if {[string match BUILD_* $nh]} {
>          set need_xen_hosts {}
> -        set need_build_host 1
> +        set need_build_host [string range $nh [expr [string first _ $nh] + 
> 1] end]

This string range stuff is rather clunky.  How about

   if {[regsub {^BUILD_(.*)} $nh need_build_host]} {

?

> -    if {$need_build_host} { catching-otherwise broken prepare-build-host }
> +    if {[llength $need_build_host]} {
> +        catching-otherwise broken {
> +            prepare-build-host-[string tolower $need_build_host]

I might be tempted to not bother with the `string tolower' and simply
let the functions have SHOUTING in their names.  Up to you.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel