httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r230592 - in /httpd/httpd/branches/2.0.x: CHANGES STATUS modules/proxy/proxy_http.c
Date Sun, 14 Aug 2005 21:03:13 GMT
On 8/6/05, William A. Rowe, Jr. <wrowe@rowe-clan.net> wrote:
> At 08:39 PM 8/6/2005, Jeff Trawick wrote:
> >On 8/6/05, wrowe@apache.org <wrowe@apache.org> wrote:
> >> Author: wrowe
> >> Date: Sat Aug  6 14:29:05 2005
> >> New Revision: 230592
> >>
> >> URL: http://svn.apache.org/viewcvs?rev=230592&view=rev
> >> Log:
> >>
> >>  As much as it pains me, seriously, it seems that reviewing the re-backport
> >>  of this code was too illegible for review, so it seems we will need to
> >>  re-review a fresh backport from httpd trunk.
> >
> >It looks to me that we have lost our second of two chances to go
> >through a stepwise, single-problem/single-solution approach to
> >resolving the issues with this code, even after multiple comments
> >stating that mixing that set of changes was undesired.  It isn't
> >impossible to move forward from this point, but I don't understand why
> >we're still in big-patch mode after those previous comments.
> 
> Hold on a second ;-)  Are you suggesting this original backport
> patch was 'small' enough for review?

I didn't realize we were talking about the original backport.  I
recall talking about a number of other seemingly-independent fixes
which were rolled together and which at least a couple of people asked
to be split up.

If during the original backport review somebody had asked me to split
up changes to solve one problem at a time, I think I would have worked
hard to do that.

Small enough for review is easy to determine empirically.

>                                                          I'd challenge that the reasons
> why the errors were not caught was that the original backport was the
> patch that was too large, and there simply weren't enough eyeballs
> on it (no insult intended; you know how many iterations my later
> changes went through.)

no insult taken

> The other side of the coin, however, is that all agree the old code
> in proxy_http is horribly broken.  If we agree trunk/ both behaves
> correctly and is now more legible, shouldn't we put far more trust
> in the code on trunk/ than the cruft in 2.0.x?

last time I played with proxy on trunk I got a crash down in
apr_reslist_something ;)  as a matter of fact I don't have a lot of
trust

> I'm suggesting that the review should have occurred commit-by-commit
> on trunk (C-t-R).  The entire history of these changes is sitting
> on trunk, with all the wrinkles from the beginning of the effort.
> 
> Since we have little faith that sufficient 'R' occurred as we moved
> through each bit of your and my efforts, perhaps it's too much to
> ask people to have faith in the resulting backport.
> 
> If I have folks look at each 'little piece', I'm happy to 'precommit'
> each of the side-effects (reformatting, apr_natstrcasecmp, etc) in
> separate commits that make the end-result patch easier to read.
> 
> But the bottom line; do you really want me to hand a "STACK" of
> patches representing each of these changes, which will inevitable
> not apply cleanly unless layered in a specific order (and even then...)
> 
> What if I take Joe's breakdown of issues and ask for each of them to
> have a separate vote (without individual patch files, the main patch
> file can be used to see 'what will change') and the moment we have
> 3 +1's I'll commit that small layer, making the final, harry refactor
> patch of the body stream/spool code just a wee bit easier to read.
> Does this appeal to everyone?

makes sense

> Forgive me if I've been humorless about this; already pointed out
> I had burned 60, maybe 80 hours of my early summer on this mess,
> all because, when I added proxy trace and tried to actually send
> bodies, the proxy request body processing would simply hang on
> the back end because the proxied request was simply mishandled.
> Apache 1.3?  Was close.  Apache 2.1?  Even closer.  2.0.x - blah.
> Fixing one small quirk would point out yet another until various
> bits of the code looked nothing as they had started as.

This seems a lot like business as usual to me.  Anybody working on 2.x
(or any big project) for a while has been through this countless
times.  What do you do?
a) Figure everything necessary to get the darn thing working.
b) for each independent fix that can be extracted, work just that fix
through the system
c) you're likely left with a larger-than-average fix (a set of changes
which make no sense without everything else in the set)

> I'll do my best to put on a smile and slog through the 'correct'
> solution in a manner 'agreeable' to both you and Joe :)

If you have enough +1s from other folks, you can remove me from the
list and I will trust the collective +1s.  In a situation where there
were not such approvals, and I wanted to play a constructive role, I
asked/complained/whatever for the changes to be submitted in a manner
that I thought I could constructively participate with.

Mime
View raw message