micro-patch for setup-storage

Thomas Neumann blacky+fai at fluffbunny.de
Mon Jun 25 22:08:26 CEST 2012


Hello Michael

Just a small suggestion. Instead of working with $_ you can directly create a variable. This way it's a bit more obvious what is going on and one doesn't provoke strange side-effects if something else is storing stuff in $_.

version 2 is even more on the 'safe side' by explicitely next-ing via a label. I like labels. Code-wise there's no difference but it's nice if the code says what it is looping over. (I hope I got it right it's a command. On first reading this code fragment I thought it was a dependency.)

tschüß
thomas, trying to wrap his head around Commands.pm::order_commands()

patch version1)

--- vanilla/Commands.pm 2012-06-25 17:43:00.000000000 +0200
+++ patched/Commands.pm 2012-06-25 21:52:08.147285563 +0200
@@ -1333,8 +1333,7 @@
     }
     my $all_matched = 1;
     if (defined($FAI::commands{$i}{pre})) {
-      foreach (split(/,/, $FAI::commands{$i}{pre})) {
-        my $cur = $_;
+      foreach my $cur (split(/,/, $FAI::commands{$i}{pre})) {
         next if scalar(grep(m{^$cur$}, @pre_deps));
         $all_matched = 0;
         last;

patch version2)

--- vanilla/Commands.pm 2012-06-25 17:43:00.000000000 +0200
+++ patched/Commands.pm 2012-06-25 22:02:37.766628161 +0200
@@ -1333,9 +1333,11 @@
     }
     my $all_matched = 1;
     if (defined($FAI::commands{$i}{pre})) {
-      foreach (split(/,/, $FAI::commands{$i}{pre})) {
-        my $cur = $_;
-        next if scalar(grep(m{^$cur$}, @pre_deps));
+      COMMAND:
+      foreach my $cur (split(/,/, $FAI::commands{$i}{pre})) {
+        if scalar(grep(m{^$cur$}, @pre_deps)) {
+          next COMMAND;
+        }
         $all_matched = 0;
         last;
       }



More information about the linux-fai mailing list