On Wed, Apr 29, 2009 at 09:12:23AM -0400, Bill Pemberton wrote:
> The script was using bareword filehandles. This is considered a bad
> practice so they have been changed to indirect filehandles.
I think this is a real improvement; using indirect filehandles mean they
get scoped properly, which can avoid errors (especially forgetting to
close() them, which happens automagically when they go out of scope).
Assuming, of course, that the scoping added by your change is correct,
and doesn't close a handle during a loop that we may have wanted to keep
open (I didn't check carefully).
But in the patch itself:
> - open(C,">",$compose_filename)
> + open my $C,'>',$compose_filename
There are actually two things happening here:
1. s/C/my $C/, which I think is good
2. losing the parentheses around open(). This is a style issue, but I
think we usually prefer the parenthesized form of most perl
builtins (and certainly in the absence of other information, it
should be left as-is).
And the style thing that probably _should_ be changed is the spacing: we
typically have a space between function arguments. So:
open(my $C, '>', $compose_file)
> - print C <<EOT;
> + print {$C} <<EOT;
Are the braces really necessary here? Is there a version of perl on
which
print $C <<EOT;
will not work?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html