> Quoting Yosef Etigin <[EMAIL PROTECTED]>: > 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 _______________________________________________ ewg mailing list ewg@lists.openfabrics.org http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ewg