Thread: Signing off of patches (was Re: Not ready for 8.3)
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
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
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)
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
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
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
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
"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
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