Return-Path: Delivered-To: apmail-apr-dev-archive@www.apache.org Received: (qmail 45491 invoked from network); 2 Jul 2009 00:35:32 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 2 Jul 2009 00:35:32 -0000 Received: (qmail 93107 invoked by uid 500); 2 Jul 2009 00:35:42 -0000 Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 93001 invoked by uid 500); 2 Jul 2009 00:35:42 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Id: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 92993 invoked by uid 99); 2 Jul 2009 00:35:42 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 02 Jul 2009 00:35:42 +0000 X-ASF-Spam-Status: No, hits=2.2 required=10.0 tests=HTML_MESSAGE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of trawick@gmail.com designates 72.14.220.158 as permitted sender) Received: from [72.14.220.158] (HELO fg-out-1718.google.com) (72.14.220.158) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 02 Jul 2009 00:35:34 +0000 Received: by fg-out-1718.google.com with SMTP id d23so1020062fga.22 for ; Wed, 01 Jul 2009 17:35:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type; bh=GWTFx3RgFkMvtfGkYIgZIRk/L9qceCjEq3P6dIAP94U=; b=kGx1ymbUQ6ud7kw8lSFXo0lOD1Y2A71EBZXo1sLRd52c0yqg1R5sCbXR3j4MQJUj25 UCIvSYIVfunBBrNxnigoduq7QrY7/eqmVd3MsRQJYxjgTOm0r0osyj04hmMKeamchfkX fQWTY1frJQ/xTLVSiGSb0wHLB9QflhAujzQQI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=R2p7ApS0d5q8QbBnWWeB0NMELerrD9w9SzqUVs6W84bbKXkKHlxyrxVjIqynuDCk44 sMCHqG4xVIgSwaJGPCYzJM8W4qdRUtaqO1jo+vWw5hk7HfPxxYWYmtgoSvQ1nu6b8zHE RRZ8JgX7XhItX6fiLk2tL69qjLlSS3zwPHetg= MIME-Version: 1.0 Received: by 10.86.90.13 with SMTP id n13mr4868649fgb.45.1246494913100; Wed, 01 Jul 2009 17:35:13 -0700 (PDT) In-Reply-To: <1246484299.5425.9.camel@shrek.rexursive.com> References: <20090630213527.8D5DB2388896@eris.apache.org> <1246484299.5425.9.camel@shrek.rexursive.com> Date: Wed, 1 Jul 2009 20:35:13 -0400 Message-ID: Subject: Re: svn commit: r789965 - in /apr/apr/trunk: CHANGES dbd/apr_dbd_pgsql.c From: Jeff Trawick To: dev@apr.apache.org Content-Type: multipart/alternative; boundary=000e0cd24a0c827ba6046dae36b5 X-Virus-Checked: Checked by ClamAV on apache.org --000e0cd24a0c827ba6046dae36b5 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On Wed, Jul 1, 2009 at 5:38 PM, Bojan Smojver wrote: > On Wed, 2009-07-01 at 08:31 -0400, Jeff Trawick wrote: > > IMO the better way to handle this would have been > > > > int ret, tmpret; > > > > and using tmpret when the function-wide ret shouldn't be touched > > > > Overloading "ret" and the semi-hidden setting of the function-wide ret > > make this code less clear than it could be. > > That is what the original patch had, in fact. I kinda liked the locally > scoped approach better, because it seemed cleaner to me. But then again, > I'm not known for very good taste. > > > Continuing down the overly picky trail: It would be better to focus > > CHANGES entries on the impact to library consumers, and omit details > > like "locally[-]scoped variables". > > OK. > > Question: was your comment essentially a -1, in which case I'll > revert/change, or was it just a remark? not a -1; just something to think about in the future --000e0cd24a0c827ba6046dae36b5 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Wed, Jul 1, 2009 at 5:38 PM, Bojan Sm= ojver <bojan@re= xursive.com> wrote:
On Wed, 2009-07-01 at 08:31 -0400, Jeff Trawick wrote: > IMO the better way to handle this would have been
>
> int ret, tmpret;
>
> and using tmpret when the function-wide ret shouldn't be touched >
> Overloading "ret" and the semi-hidden setting of the functio= n-wide ret
> make this code less clear than it could be.

That is what the original patch had, in fact. I kinda liked the local= ly
scoped approach better, because it seemed cleaner to me. But then again, I'm not known for very good taste.

> Continuing down the overly picky trail: It would be better to focus > CHANGES entries on the impact to library consumers, and omit details > like "locally[-]scoped variables".

OK.

Question: was your comment essentially a -1, in which case I'll
revert/change, or was it just a remark?

not a -1; just something to think about in the future

--000e0cd24a0c827ba6046dae36b5--