Re: [Xen-devel] [PATCH OSSTEST v2 06/11] Cope with Jessie's d-i vg name

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Wei Liu writes ("[PATCH OSSTEST v2 06/11] Cope with Jessie's d-i vg name"):
> In Jessie the default vg name is changed to "$hostname-vg". Make that
> default case and check for wheezy, squeeze and lenny for backward
> compatibility.
...
> -    our $vgname= $ho->{Name};
> +    our $vgname= $ho->{Suite} =~ m/wheezy|squeeze/
> +                 ? $ho->{Name}
> +                 : $ho->{Suite} =~ m/lenny/
> +                  ? "$ho->{Name}.$c{TestHostDomain}"
> +                  : "$ho->{Name}-vg";

This nested indentation is not conventional (in any language, AFAICT)
for sequences of ? :.

There are various acceptable indentations but I don't think this is
one of them.  (I normally prefer versions which put the condition and
the result on the same line, but that may be too tedious here.)

>  sub determine_vg_lv () {
>      $vg=
> -        $ho->{Suite} =~ m/lenny/
> -        ? "$ho->{Name}.$c{TestHostDomain}"
> -        : $ho->{Name};
> +        $ho->{Suite} =~ m/wheezy|squeeze/
> +        ? $ho->{Name}
> +        : $ho->{Suite} =~ m/lenny/
> +         ? "$ho->{Name}.$c{TestHostDomain}"
> +         : "$ho->{Name}-vg";

Now, admittedly, this logic was duplicated before you started.  But
really I think you should abolish the duplication before extending it.

Thanks,
Ian.

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