This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Flatten sysdeps/unix/bsd/bsd4.4 into sysdeps/unix/bsd
On Wed, 9 May 2012, David Miller wrote:
> From: Roland McGrath <roland@hack.frob.com>
> Date: Wed, 9 May 2012 12:59:09 -0700 (PDT)
>
> >> I consider it important to make it easier for people to find their way
> >> around the source tree.
> >
> > This particular set of changes is quite marginal in that regard.
>
> It's one of many steps to get to an intended destination, in that
> regard all such changes are important and anything but marginal.
Indeed.
More generally: it's important to deal with *any* patch while it's still
fresh in the submitter's mind and before the source tree diverges from the
state on which the patch was based. Patches shouldn't be gated on
something that *might* happen at some unknown point in the future (such as
Hurd support being made to work), or on some general cleanup substantially
unrelated to the subject matter of the patch, or on any one particular
person having time to review a patch where there are other people with
appropriate expertise to do so as well. (It's good to comment on how such
a cleanup might make things nicer, but if the submitter considers that a
distraction then the result should be to track the desired cleanup
somewhere and consider the patch on its own merits without requiring the
cleanup.)
I've suggested cleanups myself in patch review without being clear about
what is or is not required. I think we all need to get into a better
habit of saying such things as:
* This might more cleanly be done after general cleanup X, *but given the
existing similar issues it's OK the way you did it*; I'll file a bug for
the cleanup.
* Do you see an easy way to do this while avoiding issue Y? If not, it's
OK as is, but file a bug for the general problem.
* Can someone test this on platform X within the next couple of days? OK
after then in the absence of negative test results.
and not (or rarely):
* Cleanup X would be better. [Without explicitly saying whether it's OK
as is.]
* You should do this general infrastructural cleanup first.
* This needs testing on platform X.
Thoughts from other people? Should we have something on the wiki about
good and bad patterns of patch review?
--
Joseph S. Myers
joseph@codesourcery.com