httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: svn commit: r230592 - in /httpd/httpd/branches/2.0.x: CHANGES STATUS modules/proxy/proxy_http.c
Date Tue, 09 Aug 2005 01:34:24 GMT
At 06:58 PM 8/8/2005, Roy T. Fielding wrote:
>Bill, if you spent more time making your changes understandable
>by documenting what they change instead of various random things
>totally unrelated to each patch, then maybe people like me could
>review them.

Without quoting you at length and spending major bandwidth; 
generally I agree; however the existing code needed major surgery.
A victim myself, I'm glad my surgeons have left very clean and
minimal scars; looks like a good arm/finger/leg/unmetionables.
I look at code the same way, remember I jumped in around 1.3.9.
Alot of the code by that point was illegible beyond belief.  By
the time we were done with arguments, most of my fixes were
ultimately adopted.  That represented about 500 hours of grief.
The resulting code was legible.  Not clean, but better.

Fixing these bugs in isolation (just eliminate every patch that
said 'whitespace only', 'no effect', etc) propagates the very
same crap I just wasted my time to fix, and leaves nothing
for the dev behind me to grab on to.

If I need to drop in int i = foo; i += offset; memset(i, n);  and
the i was never declared, it's possible to do this midstream with
an {} section.  Sure, that's a minimal patch.  Is it elegant?
Do we need the extra indention (can we even tolerate it with 80
col considerations?)  Is it clear to the reader of the resulting
code?  I argue that most of our reviewers are looking at code that
isn't working, not devs reading this list.

These are the points we agree and disagree on.  Yes, my patch seeks
to make the end-result comparable to httpd/trunk - and you have no
interest in considering that case.  You want simple.  So just grab
the branch and see the commits which don't say "whitespace only"
or "noop, cleaning up ...".  I was especially careful to consider
that both devs who want the straight line, and devs who want to
know where we are, both can accomplish their goals.

There is no way in hell I'm refactoring now near 90 hours of work
to make it more digestable.  This big steak I just chowed down
on Sunday, regurgitating into tiny little digestable pieces for 
chicks to lap up and swallow, is absolutely illustrative of what
the hell the code should look like.  You don't like #7, Joe doesn't 
like #3, Jeff doesn't like #13 and Jim doesn't like #10.  For that
matter I can't even stand where it started from or how I got to the
end.  That's fine, take each little digestable bit.  If it doesn't 
break the code, choose to not vote or +1 it.  I don't care.  If it 
breaks the code, give me technical justification and I'll address 
the complaint.

I'M FUCKING SICK AND TIRED OF BEING ASKED TO BRING EACH SPECIFIC
REVIEWIER THEIR OWN SHINEY FUCKING ROCK.  TAKE THIS FUCKING PATCH
AND SHOVE IT WHEREVER THE FUCK YOU WANT.  (Oh, but be certain you
have a long flamewar and debate over the exact composition of the
rocks, the color of the rocks, the diversity and exclusivity of 
specific rocks over the others, and don't forget to spend the next
20 years debating the specific metallurgy and carbon dating of the
rocks, arguing over the hand of a maker or natural selection.)

Existing code was 'too broke', so it requires a whole lot of
fixes to all the misbehavior,and a whole lot of fixes to make
the resulting code legible..  By existing, I'm not implicating
Jeff's patch; the origin code we both started with, post 
refactor from 1.3, was the issue (heck, probably before then.)  
I'm shocked it was voted in without any additional cleanup.
Even more scary, my name might even be on that +1 roster.

Addressing your other issue, when I take more than 3 (read: 4)
hits at a section of code; I'm applying layered multiple bug
fixes with different effects, not fixing my own.  If I have 
to fix my own code, I hit that in the next commit.

Well, at least one can hope that's the way it goes.

Bill



Mime
View raw message