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

Reply via email to