[ewg] Re: RFC: OFED-1.3-20070823-1130 - first build

[Michael S. Tsirkin]

> Quoting Yosef Etigin <yosefe at voltaire.com>:
> Subject: Re: RFC: OFED-1.3-20070823-1130 - first build
> 
> Hi Vlad,
> 
> I have some comments regarding install.pl.
> Overall, I think it's too long for a perl script.

So ... what's your point?

> 1. The first ~1K lines are a database of the existing packages.
>    It has some unneccesary initiallizations:
>      selected => 0, installed => 0, rpm_exist => 0, rpm_exist32 => 0

I agree here.

>     In my opinion, this database could exist an an external XML file,
>    rather easy to parse that with perl.

It's hard to see what inventing yet another format would buy us.
Let's keep it simple.

> 2. How about doing "a ? b : c" instead of "if (a) { b } else { c }" ? 

Looks like a matter of style.

> 3. There are some copy-and-paste blocks.. for example, in select_packages():
> 
> instead of:
> 	if ($package eq "mvapich2_conf_impl") {
> 		$mvapich2_conf_impl = $selected;
> 		next;
> 	}
> 	elsif ...
> write:
> 	if ($package =~ /^mvapich2_conf_/) {
> 		$$package = $selected;
> 		next;
> 	}
> 
>    same for the stuff in set_compilers()
> 
> 4. Instead of print RED "...", RESET "\n"; exit 1, you could do smth like error()
>    since redirecting this to files causes some mess
> 
> 5. instead of iterating over arrays and checking conditions you
>    could use grep, map, and such.

Could. But shouldn't.
Simple loops are much easier to understand.

-- 
MST