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: [PATCH] APR thread updates and associated httpd-2.0 changes
Date Sat, 21 Jul 2001 05:38:04 GMT
From: "Aaron Bannert" <aaron@ebuilt.com>
Sent: Friday, July 20, 2001 11:57 PM


> On Fri, Jul 20, 2001 at 09:20:49PM -0700, Ryan Bloom wrote:
> >
> > MNSO is that the goal of
> > patches should be to change as little code as possible.  The more changes there
are,
> > the harder it is to review the patch.  If a patch is generated, and there isn't
a need for
> > a line of code to have changed, the person who created the patch should go through
> > and ask why that line was changed.
> 
> Your inspection is valid, but I'm not sure it is in the best interest of
> the commiters on this list for me to be supplying many smaller and less
> important patches all day long. 

It is _always_ in the best interest (of you, and reviewers) to keep them very,
very short and sweet.

I spent a long time sending about four very large, and 'untrusted' patches
to the list.  They were never applied.  I stopped, looked at the very specific
things I wanted accomplished, and those patches were applied to httpd (1.3.12
at the time) very quickly.

The simple fact is, if it is obvious what you are trying to accomplish, and you
are only changing 'one thing', it is trivial for a committer to apply the patch.

If you change a dozen things (even name changes mixed with functional changes
mixed with retabulation or spacing adjustments) then it takes a great deal of
anyone's time to absorb it, and then determine what is right or wrong with it.

> The code-test-commit cycle is just way to long for that. 

A five line change that is obvious will be committed within hours.  Do that three
times a day and the patch that takes weeks to argue and be commited happens in a 
matter of days.

> In any case, I will try to keep the patches short and
> sweet in the future.

Please do :)

> As for updating this patch, the only problem that I see is there was a
> variable left over that is no longer is use, and it's declaration should
> be removed. Do you still want me to revert all the 'thread' identifiers
> back to '(*new)'??

Of course!  And offer up a seperate patch to change *only* the name of these
less-than-intuitive identifiers.  Rbb is the last person who will ever try to
argue that his identifiers are the 'one true correct name' for anything :)

Change one bit at a time, to one end, and your patches can happen much more
quickly.  While the rest of us are far from perfect in this, we really do try,
as committers, to break up formatting changes from functional changes, and our
functional changes into parsable units.

Bill


Mime
View raw message