Return-Path: 
 <dev-return-66165-apmail-httpd-dev-archive=httpd.apache.org@httpd.apache.org>
Delivered-To: apmail-httpd-dev-archive@www.apache.org
Received: (qmail 71586 invoked from network); 4 Oct 2009 11:46:42 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3)
  by minotaur.apache.org with SMTP; 4 Oct 2009 11:46:42 -0000
Received: (qmail 68517 invoked by uid 500); 4 Oct 2009 11:46:41 -0000
Delivered-To: apmail-httpd-dev-archive@httpd.apache.org
Received: (qmail 68451 invoked by uid 500); 4 Oct 2009 11:46:41 -0000
Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
list-help: <mailto:dev-help@httpd.apache.org>
list-unsubscribe: <mailto:dev-unsubscribe@httpd.apache.org>
List-Post: <mailto:dev@httpd.apache.org>
List-Id: <dev.httpd.apache.org>
Delivered-To: mailing list dev@httpd.apache.org
Received: (qmail 68442 invoked by uid 99); 4 Oct 2009 11:46:41 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 04 Oct 2009 11:46:41 +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 (nike.apache.org: domain of trawick@gmail.com designates
 209.85.220.227 as permitted sender)
Received: from [209.85.220.227] (HELO mail-fx0-f227.google.com)
 (209.85.220.227)
    by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 04 Oct 2009 11:46:31 +0000
Received: by fxm27 with SMTP id 27so1972641fxm.41
        for <dev@httpd.apache.org>; Sun, 04 Oct 2009 04:46:11 -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=KVSwNgyggU3VtSYWyOAvQgMeaG4ultjuuiLoXBPTWhw=;
        b=C7dnjyVzIt36a50eJeZWu83uHf6rouRyoBk+iptS0+d5pt1IR+qc1iOQTipLMAXVa0
         WOXE98fOquQWHperlxBcX3NKAzKDJ7/4cbvVp/J1vvb4P7lt3jwd6NVWUQ2gjmsZVj/h
         uhFO3f/2XojUD8RsZ52zlkuYEDSAuLN/BSXm4=
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=RRC7YTj7eOGdAJ71AcwGzKOs64a3ZZy2NslEf/RrBZFN+delXgLndRaPfCMpP2KWtT
         xK4Qt2L/QA+h8azPZdWyXMR9xsRCcg/nGk+MhXBMNMdkFL3bQSr/3afvN0taMLbRVD63
         YC+cu4pI/2grozIIdlojg3/S7mfzDlzHNxmdw=
MIME-Version: 1.0
Received: by 10.86.187.7 with SMTP id k7mr4223444fgf.30.1254656771554;
 Sun, 04
	Oct 2009 04:46:11 -0700 (PDT)
In-Reply-To: <4AC7EFED.2060701@sharp.fm>
References: <20091003145400.B562E23888E4@eris.apache.org>
	 <cc67648e0910030920l69b725aboab205885d36aaa4a@mail.gmail.com>
	 <4AC7EFED.2060701@sharp.fm>
Date: Sun, 4 Oct 2009 07:46:11 -0400
Message-ID: <cc67648e0910040446x58c94757v5b5c5382bdc5543@mail.gmail.com>
Subject: Re: svn commit: r821333 - in /httpd/httpd/trunk: CHANGES
	docs/manual/mod/mod_cache.xml modules/cache/cache_util.c
From: Jeff Trawick <trawick@gmail.com>
To: dev@httpd.apache.org
Content-Type: multipart/alternative; boundary=00163600d59d2efe1c04751a8bdd
X-Virus-Checked: Checked by ClamAV on apache.org

--00163600d59d2efe1c04751a8bdd
Content-Type: text/plain; charset=ISO-8859-1

On Sat, Oct 3, 2009 at 8:44 PM, Graham Leggett <minfrin@sharp.fm> wrote:

> Jeff Trawick wrote:
>
>     +  *) mod_cache: Fix uri_meets_conditions() so that CacheEnable will
>>    +     match by scheme, or by a wildcarded hostname. PR 40169
>>    +     [Ryan Pendergast <rpender us.ibm.com <http://us.ibm.com>>,
>>    Graham Leggett]
>>    +
>>
>>
>> I guess it is "Submitted:" by both you and Ryan?  (Commit log doesn't
>> match CHANGES?)
>>
>> This is curious because this patch looks most like one attached to the
>> issue by Peter Grandi.  The last patch attached by Ryan is a one-liner, and
>> now marked obsolete.
>>
>
> I mixed up the names, thanks for the catch.
>
> The patch is different enough that if I was the original contributor, I'd
> say "oi, that isn't my code any more, and the person who reviewed it changed
> it, so don't blame me for their bugs".
>
> CHANGES doesn't make distinctions about who did what to code, it just lists
> the people responsible, usually in the order of contribution.
>
> The commit log makes a clearer distinction that the code was originally
> submitted, and then reviewed and modified by me. They do not contradict one
> another.


IMO the "modified by me" part is lost.


>
>
>  some aspects of this patch change the style or handle certain issues that
>> are ignored everywhere else
>>
>
> That's because the entire afternoon I devoted to this patch was spent
> painstakingly testing each path through the code, to make 100% sure that it
> worked both for the forward and reverse proxy case (the original patch
> didn't work in the reverse proxy case).
>
> The original patch didn't follow the style of mod_cache at all, and I
> changed the majority of it to match. Obviously I didn't catch all of it, but
> then I prioritised whether it worked or not over and above where a brace was
> placed.
>
>  we have unused parameters all over the place (e.g., with hook functions);
>> why we need to do "(void) s;" here?
>>
>
> Why are you asking why?
>
> If it's wrong, highlight it, we fix it, we move on.


My gut instinct when I see something odd is that I'd like to know what that
was for.  Yeah, I'm pretty sure it is wrong and needs to be removed, but I
wondered why as well.  I suppose I could have said "I don't see any purpose
for this, so delete."

>
>  also, since no debug provision utilizing s was committed, the developer
>> has to add code anyway when wanting to log something; can't they just add s
>> at that time instead of leaving it in the code?  it should take all of 15
>> seconds to do that
>>
>
> Did you think I just took the patch as it was and applied it blindly to
> httpd-trunk and committed it?
>
>
What I think is that you are not comfortable having a civil conversation
about your commits.

Would "Please delete this unnecessary code" have been better received than
explaining why I thought it was preferable to leave that minimal debug
support out?



> The fact that you can only find issues with the style, and not with the
> functionality indicates that the afternoon's work was a success.
>

Good for you for moving an outside patch forward; that's great, of course.


>
> It's late, I'm tired and I'm off to bed. The style can be fixed tomorrow.
>

It will be appreciated.

--00163600d59d2efe1c04751a8bdd
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div class=3D"gmail_quote">On Sat, Oct 3, 2009 at 8:44 PM, Graham Leggett <=
span dir=3D"ltr">&lt;<a href=3D"mailto:minfrin@sharp.fm">minfrin@sharp.fm</=
a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"border-l=
eft: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left:=
 1ex;">
Jeff Trawick wrote:<br>
<br>
<blockquote class=3D"gmail_quote" style=3D"border-left: 1px solid rgb(204, =
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class=3D"im"=
>
 =A0 =A0+ =A0*) mod_cache: Fix uri_meets_conditions() so that CacheEnable w=
ill<br>
 =A0 =A0+ =A0 =A0 match by scheme, or by a wildcarded hostname. PR 40169<br=
></div>
 =A0 =A0+ =A0 =A0 [Ryan Pendergast &lt;rpender <a href=3D"http://us.ibm.com=
" target=3D"_blank">us.ibm.com</a> &lt;<a href=3D"http://us.ibm.com" target=
=3D"_blank">http://us.ibm.com</a>&gt;&gt;,<div class=3D"im"><br>
 =A0 =A0Graham Leggett]<br>
 =A0 =A0+<br>
<br>
<br>
I guess it is &quot;Submitted:&quot; by both you and Ryan? =A0(Commit log d=
oesn&#39;t match CHANGES?)<br>
<br>
This is curious because this patch looks most like one attached to the issu=
e by Peter Grandi. =A0The last patch attached by Ryan is a one-liner, and n=
ow marked obsolete.<br>
</div></blockquote>
<br>
I mixed up the names, thanks for the catch.<br>
<br>
The patch is different enough that if I was the original contributor, I&#39=
;d say &quot;oi, that isn&#39;t my code any more, and the person who review=
ed it changed it, so don&#39;t blame me for their bugs&quot;.<br>
<br>
CHANGES doesn&#39;t make distinctions about who did what to code, it just l=
ists the people responsible, usually in the order of contribution.<br>
<br>
The commit log makes a clearer distinction that the code was originally sub=
mitted, and then reviewed and modified by me. They do not contradict one an=
other.</blockquote><div><br>IMO the &quot;modified by me&quot; part is lost=
.<br>
=A0<br></div><blockquote class=3D"gmail_quote" style=3D"border-left: 1px so=
lid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div=
 class=3D"im"><br>
<br>
<blockquote class=3D"gmail_quote" style=3D"border-left: 1px solid rgb(204, =
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
some aspects of this patch change the style or handle certain issues that a=
re ignored everywhere else<br>
</blockquote>
<br></div>
That&#39;s because the entire afternoon I devoted to this patch was spent p=
ainstakingly testing each path through the code, to make 100% sure that it =
worked both for the forward and reverse proxy case (the original patch didn=
&#39;t work in the reverse proxy case).<br>

<br>
The original patch didn&#39;t follow the style of mod_cache at all, and I c=
hanged the majority of it to match. Obviously I didn&#39;t catch all of it,=
 but then I prioritised whether it worked or not over and above where a bra=
ce was placed.<div class=3D"im">
<br>
<br>
<blockquote class=3D"gmail_quote" style=3D"border-left: 1px solid rgb(204, =
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
we have unused parameters all over the place (e.g., with hook functions); w=
hy we need to do &quot;(void) s;&quot; here?<br>
</blockquote>
<br></div>
Why are you asking why?<br>
<br>
If it&#39;s wrong, highlight it, we fix it, we move on.</blockquote><div><b=
r>My gut instinct when I see something odd is that I&#39;d like to know wha=
t that was for.=A0 Yeah, I&#39;m pretty sure it is wrong and needs to be re=
moved, but I wondered why as well.=A0  I suppose I could have said &quot;I =
don&#39;t see any purpose for this, so delete.&quot;</div>
<blockquote class=3D"gmail_quote" style=3D"border-left: 1px solid rgb(204, =
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class=3D"im"=
><br>
<blockquote class=3D"gmail_quote" style=3D"border-left: 1px solid rgb(204, =
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
also, since no debug provision utilizing s was committed, the developer has=
 to add code anyway when wanting to log something; can&#39;t they just add =
s at that time instead of leaving it in the code? =A0it should take all of =
15 seconds to do that<br>

</blockquote>
<br></div>
Did you think I just took the patch as it was and applied it blindly to htt=
pd-trunk and committed it?<br>
<br></blockquote><div><br>What I think is that you are not comfortable havi=
ng a civil conversation about your commits.<br><br>Would &quot;Please delet=
e this unnecessary code&quot; have been better received than explaining why=
 I thought it was preferable to leave that minimal debug support out?<br>
<br>=A0</div><blockquote class=3D"gmail_quote" style=3D"border-left: 1px so=
lid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
The fact that you can only find issues with the style, and not with the fun=
ctionality indicates that the afternoon&#39;s work was a success.<br></bloc=
kquote><div><br>Good for you for moving an outside patch forward; that&#39;=
s great, of course.<br>
=A0<br></div><blockquote class=3D"gmail_quote" style=3D"border-left: 1px so=
lid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
It&#39;s late, I&#39;m tired and I&#39;m off to bed. The style can be fixed=
 tomorrow.<br></blockquote><div><br>It will be appreciated.<br><br></div></=
div>

--00163600d59d2efe1c04751a8bdd--