Thread: Signing off of patches (was Re: Not ready for 8.3)

Signing off of patches (was Re: Not ready for 8.3)

From
Alvaro Herrera
Date:
Alvaro Herrera wrote:

> Sorry, I wasn't trying to say that we should follow the Linux model all
> that closely.  I know there are regressions in the "point zero"
> releases, and that there are bugs.

This morning a friend IM'ed me a comment about Martin Michlmayr's PhD
thesis, which is about release process on open source projects.  It is
an interesting read:

http://www.cyrius.com/journal/phd?reverse=yes

Now, I don't think we can apply any conclusions directly from his study.
But it got me thinking anyway; what can we learn from them?  I don't
think time-based release is appropriate for us, because we already have
the time constraints, but it is not good because we allow some patches
to "starve" on patch queues, or we let the schedule slip.

Having the Linux process still in memory, I started thinking that maybe
what we need, is a sign-off process, whereby developer A reviews other
developers' patches, make comments, and when the commented-on developer
(call him B) has fixed the issues that A had, then A signs off B patch.
In return for the favor, B also reviews and signs off A patches.
Eventually this leads to more cooperation between otherwise independent
developers.

The point of all this is to have an incentive for developers to review
patches.  And what better incentive than having a chance that other
developers will retribute by reviewing his own patches?

This doesn't need to be anything too formal; I think that just having
people feel that other people at least took a look at the thing and
there are no obvious, glaring mistakes could go a long way.  (I have
this weird idea that I should not apply a patch unless someone else says
"hey, looks OK to me".  Somehow, the mere lack of objections does not
increase my confidence.)

Opinions?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Signing off of patches (was Re: Not ready for 8.3)

From
Andrew Sullivan
Date:
On Fri, May 18, 2007 at 12:05:55PM -0400, Alvaro Herrera wrote:

> there are no obvious, glaring mistakes could go a long way.  (I have
> this weird idea that I should not apply a patch unless someone else says
> "hey, looks OK to me".  Somehow, the mere lack of objections does not
> increase my confidence.)

I have nothing to contribute on the suggestion, since I can neither
offer review nor patches.  But I can offer an analogy that will maybe
strengthen your point, and might offer a hint of how to make the
developer community bigger.

In the IETF working groups I follow, most of the chairs have decided
to impose some baseline level of group review for protocol documents. 
In dnsop, for instance, we have a rule that if at least five people
do not review an Internet Draft and agree to its publication, it just
won't get advanced as a working group document.  The idea is that, if
we can't get that small number of reviews, then either the working
group either isn't interested in the feature or topic, or the draft
is a bad idea as it stands.  

As a result, if you want to have the suasion to get people to review
your own submissions, you also have to do the work of reviewing
others'.  But it also means that if you're new to an area, you can
become better in that area by doing document review.  Probably, your
own reviews won't uncover big flaws that those more experienced with
the protocol will find; but you'll be able to make some small
contributions that will allow you help in getting the documents
finished.  Also, while you're at it, you'll be forced to read all the
referenced documents, which help you learn about the protocol and
therefore make you more valuable to the WG.

Perhaps, then, new contributors to Postgres could also take on the
task of reviewing some of the patches, not as a matter of being the
_only_ reviewer -- the new code still needs review by those more
experienced with the rest of the code -- but as a first-pass review
that will help in a "more eyeballs" sort of way.  This would also
have the happy paedogogical effect that those newer reviewers would
learn more of the code in each cycle.  I think this is similar to a
previous suggestion someone made about "mentored review", but it
doesn't require formal mentoring for it to get started.

A

-- 
Andrew Sullivan  | ajs@crankycanuck.ca
Users never remark, "Wow, this software may be buggy and hard 
to use, but at least there is a lot of code underneath."    --Damien Katz


Re: Signing off of patches (was Re: Not ready for 8.3)

From
Andrew Dunstan
Date:

Alvaro Herrera wrote:
>
> Having the Linux process still in memory, I started thinking that maybe
> what we need, is a sign-off process, whereby developer A reviews other
> developers' patches, make comments, and when the commented-on developer
> (call him B) has fixed the issues that A had, then A signs off B patch.
> In return for the favor, B also reviews and signs off A patches.
> Eventually this leads to more cooperation between otherwise independent
> developers.
>
>
>   

You're thinking of the wrong process. What you seem to be suggesting is 
a process that mimics the Usenet Oracle[1], in which if you ask a 
question you get given one to answer ...

cheers

andrew

[1] http://cgi.cs.indiana.edu/~oracle/index.cgi (Zot)


Re: Signing off of patches (was Re: Not ready for 8.3)

From
Alvaro Herrera
Date:
Andrew Dunstan wrote:
> 
> Alvaro Herrera wrote:
> >
> >Having the Linux process still in memory, I started thinking that maybe
> >what we need, is a sign-off process, whereby developer A reviews other
> >developers' patches, make comments, and when the commented-on developer
> >(call him B) has fixed the issues that A had, then A signs off B patch.
> >In return for the favor, B also reviews and signs off A patches.
> >Eventually this leads to more cooperation between otherwise independent
> >developers.
> 
> You're thinking of the wrong process. What you seem to be suggesting is 
> a process that mimics the Usenet Oracle[1], in which if you ask a 
> question you get given one to answer ...

Interesting, but, err, not really what I was thinking.

In order to make patch review more effective, perhaps we could use some
tools to help us.  How about
http://www.chipx86.com/blog/?p=222
?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Signing off of patches (was Re: Not ready for 8.3)

From
Tom Lane
Date:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> In order to make patch review more effective, perhaps we could use some
> tools to help us.  How about
> http://www.chipx86.com/blog/?p=222

I kinda think this is emphasizing the wrong end of the process.  "Code
everything, then ask for comments" is about as far away from the correct
mindset as one can readily get.

I'd like to see something that emphasizes review and feedback at the
stages of germinal idea, rough functional spec, implementation concept,
etc.  Reviewing the actual code is good too, but if that's the first
stage that you ask for help at, you are off in the wrong direction;
particularly so if you're a larval Postgres hacker.
        regards, tom lane


Re: Signing off of patches (was Re: Not ready for 8.3)

From
"Karl O. Pinc"
Date:
On 05/18/2007 08:59:11 PM, Tom Lane wrote:

> I'd like to see something that emphasizes review and feedback at the
> stages of germinal idea, rough functional spec, implementation
> concept,
> etc.  Reviewing the actual code is good too, but if that's the first
> stage that you ask for help at, you are off in the wrong direction;
> particularly so if you're a larval Postgres hacker.

Speaking as a larval Postgres hacker I have trouble asking about
the germinal idea and rough functional spec parts.  Without
having some clue about the implementation concept it's
difficult for me to imagine whether or not I want to
or will be able to put the effort into making the actual
code work.  Rather then spend time yakking about design,
I figured I'd better put the effort into poking about the
code enough to see what would be involved.

It might be different if I "just felt like hacking
about in Postgres", but I've got a specific need I want
to see addressed.  That constrains what I'm willing to
work on so the pertinent question becomes how hard it's
going to be to get the job done.  I guess what it comes down
to is that I trust my estimate of how hard it's going to
be for me to learn to do something new, even a bad initial
estimate, more than I trust some stranger who's not familiar
with my skill set.  So I try cutting some code just to get
going.  Then decide if I want to continue.

Obviously a PG expert is going to be able to tell me a lot
about what I'm trying to do.  But that might take a lot of
mentoring, and I'm also not ready to commit to paying back
with code contribution that goes past my immediate need.
Which makes me reluctant to ask for such help.

I'd guess a lot of larval hackers are in my shoes.  They're
getting involved to address some specific need.

Hope this helps.


Karl <kop@meme.com>
Free Software:  "You don't pay back, you pay forward."                 -- Robert A. Heinlein



Re: Signing off of patches (was Re: Not ready for 8.3)

From
Neil Conway
Date:
On Fri, 2007-18-05 at 21:59 -0400, Tom Lane wrote:
> I kinda think this is emphasizing the wrong end of the process.

I don't disagree, but I think a tool like this would still be enormously
helpful (to me, at any rate). While there's more to the process of
feature development than just mailing patch hunks back and forth, Review
Board seems a nice improvement for that part of the process.

-Neil




Re: Signing off of patches (was Re: Not ready for 8.3)

From
Tom Lane
Date:
"Karl O. Pinc" <kop@meme.com> writes:
> On 05/18/2007 08:59:11 PM, Tom Lane wrote:
>> I'd like to see something that emphasizes review and feedback at the
>> stages of germinal idea, rough functional spec, implementation
>> concept,

> Speaking as a larval Postgres hacker I have trouble asking about
> the germinal idea and rough functional spec parts.  Without
> having some clue about the implementation concept it's
> difficult for me to imagine whether or not I want to
> or will be able to put the effort into making the actual
> code work.

Well, but if you ask at an early stage it's perfectly fair to ask for
comments on how much work an implementation of idea X might be.  Plus
people could save you from wasting time going down dead-end paths.
        regards, tom lane


Re: Signing off of patches (was Re: Not ready for 8.3)

From
"Karl O. Pinc"
Date:
On 05/19/2007 12:48:22 PM, Tom Lane wrote:
> Well, but if you ask at an early stage it's perfectly fair to ask for
> comments on how much work an implementation of idea X might be.  Plus
> people could save you from wasting time going down dead-end paths.

True.  But then I wouldn't get extra points for being both clueless
_and_ stubborn.  ;-)


Karl <kop@meme.com>
Free Software:  "You don't pay back, you pay forward."                 -- Robert A. Heinlein