Date: Thu, 16 Apr 2020 23:56:49 -0700
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: cleaning perl code
Message-ID: <20200417065649.GC1061007@rfd.leadboat.com>
References: <20200414204432.GA22922@alvherre.pgsql>
 <86b0e20d-efc1-4169-aeb1-be49b3a375ab@2ndQuadrant.com>
 <20200416030101.GB977691@rfd.leadboat.com>
 <ee6dff48-485d-974d-cc68-67ee86fe73b8@2ndQuadrant.com>
 <4426.1587045226@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4426.1587045226@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Thu, Apr 16, 2020 at 09:53:46AM -0400, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> > On 4/15/20 11:01 PM, Noah Misch wrote:
> >> It would be an unpleasant surprise to cause a perlcritic buildfarm failure by
> >> moving a function, verbatim, from a non-strategic file to a strategic file.
> >> Having two Perl style regimes in one tree is itself a liability.
> 
> > Honestly, I think you're reaching here.
> 
> I think that argument is wrong, actually.  Moving a function from a single
> use-case into a library (with, clearly, the intention for it to have more
> use-cases) is precisely the time when any weaknesses in its original
> implementation might be exposed.  So extra scrutiny seems well warranted.

Moving a function to a library does call for various scrutiny.  I don't think
it calls for replacing "no warnings;" with "no warnings; ## no critic", but
that observation is subordinate to your other point:

> Whether the "extra scrutiny" involved in perlcritic's higher levels
> is actually worth anything is a different debate, though, and so far
> it's not looking like it's worth much :-(

Yeah, this is the central point.  Many proposed style conformance changes are
(a) double-entry bookkeeping to emphasize the author's sincerity and (b) regex
performance optimization.  Those are not better for libraries than for
non-libraries, and I think they decrease code quality.

Even if such policies were better for libraries, the proposed patch applies
them to .pm files with narrow audiences.  If DBD::Pg were in this tree, that
would be a different conversation.