httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <>
Subject Re: svn commit: r821333 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_util.c
Date Sun, 04 Oct 2009 13:22:53 GMT
Jeff Trawick wrote:

> My gut instinct when I see something odd is that I'd like to know what
> that was for.

First off, I am not in a position to tell you why it was done like that,
 that came from the original contributor, so I don't know why you were
asking me. Although having looked at it it is pretty obvious why.

The original author had added ap_log_error() into the code, and when
they were done they removed them again, but forgot to remove the
server_rec they had wired through that made ap_log_error() possible.

And it's pretty obvious how it could be missed by me. The comment at the
top clearly explained why the server_rec had been added. I expected to
see ap_log_error() further down, but instead got distracted for a few
hours by the fact the patch had completely rewritten the function and in
its original form didn't fully work.

And that was fine too, because you picked it up, creating the
opportunity for it to be fixed.

> 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."

Just flag it to be looked at further, as in "you missed a spot".

> 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?

It would have, yes.

Rhetorical questions come across as "how could you have missed that
moron" even though that may not have been the original intention.

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

Fixed in r821538.


View raw message