On 4/12/20 3:42 AM, Noah Misch wrote: > On Sat, Apr 11, 2020 at 12:13:08PM -0400, Andrew Dunstan wrote: >> --- a/src/tools/msvc/Project.pm >> +++ b/src/tools/msvc/Project.pm >> @@ -420,13 +420,10 @@ sub read_file >> { >> my $filename = shift; >> my $F; >> - my $t = $/; >> - >> - undef $/; >> + local $/ = undef; >> open($F, '<', $filename) || croak "Could not open file $filename\n"; >> my $txt = <$F>; >> close($F); >> - $/ = $t; > +1 for this and for the other three hunks like it. The resulting code is > shorter and more robust, so this is a good one-time cleanup. It's not > important to mandate this style going forward, so I wouldn't change > perlcriticrc for this one. > >> --- a/src/tools/version_stamp.pl >> +++ b/src/tools/version_stamp.pl >> @@ -1,4 +1,4 @@ >> -#! /usr/bin/perl -w >> +#! /usr/bin/perl >> >> ################################################################# >> # version_stamp.pl -- update version stamps throughout the source tree >> @@ -21,6 +21,7 @@ >> # >> >> use strict; >> +use warnings; > This and the other "use warnings" additions look good. I'm assuming you'd > change perlcriticrc like this: > > +[TestingAndDebugging::RequireUseWarnings] > +severity = 5
OK, I've committed all that stuff. I think that takes care of the non-controversial part of what I proposed :-) cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services