Re: Patch application

Tom Lane <tgl@sss.pgh.pa.us> writes:

> Ian Lance Taylor <ian@airs.com> writes:
> > In projects like gcc and the GNU binutils, we use a MAINTAINERS file.
> > Some people have blanket write privileges.  Some people have write
> > priviliges to certain areas of the code.  Anybody else needs a patch
> > to be approved before they can check it in.  Patches which are
> > ``obviously correct'' are always OK.
> 
> Would you enlarge on what that fourth sentence means in practice?
> 
> Seems like the sticky issue here is what constitutes "approval".
> We already have a policy that changes originating from non-committers
> are supposed to be reviewed before they get applied, but what Bruce
> is worried about is the quality of the review process.

In practice, what it means is that somebody who has a patch, and does
not have the appropriate privileges, sends it to the mailing list.
Most patches apply to a single part of the code.  The person
responsible for that part of the code, or a person with blanket write
privileges, reviews the patch, and approves it, or denies it, or
approves it with changes.

If approved, and the original submitter has write privileges, he or
she checks it in.  Otherwise, the maintainer who did the review checks
it in.

If approved with changes, and the original submitter has write
privileges, he or she makes the changes and checks it in.  Otherwise
he or she makes the changes, sends them back, and the maintainer who
did the review checks it in.

One advantage of the MAINTAINERS file with respect to the review
process is that it tells you who knows most about particular areas of
the code.  For example, in the gcc MAINTAINERS file I sent earlier,
there are people with blanket write privileges who are also listed as
responsible for particular areas of gcc.

Another advantage is that it reduces load on the maintainers, since
many people can check in their own patches.  Since there are many
people, some sort of control is needed.

The goal is not excessive formalism; as I said, there is nothing which
actually prevents anybody with write privileges from checking in a
patch to any part of the code.  The goal is to guide people to the
right person to approve a particular patch.

Ian