I think this new subroutine could use some refactoring: 30 sub update { 31 my $prev = _get_revision(); 32 my $revision = _analyze_sandbox(); 33 if (defined ($prev) && ($revision ne $prev)) { 34 $revision = 'unknown' unless defined $revision; 35 eval { 36 open my $FH, ">", $cache; 37 print $FH "$revision\n"; 38 close $FH; 39 $current = $revision; 40 }; 41 } 42 }
Line 39 does not need to be inside the eval{}. If we pull it out, then we can, and should refactor the eval{} into an internal subroutine, as it would then exactly match this code farther down: 58 eval { 59 open my $FH, ">", $cache; 60 print $FH "$revision\n"; 61 close $FH; 62 }; The control flow here is somewhat awkward and creates additional branches which will need coverage in testing (cf t/configure/017 and 018): 33 if (defined ($prev) && ($revision ne $prev)) { 34 $revision = 'unknown' unless defined $revision; If no one gets to this today I will try to work on this this evening. kid51