Re: Patch review process

Marty Connor <[email protected]> Thu, 9 Mar 2006 13:25:15 -0500
Newsgroups gmane.network.etherboot.devel
Message-ID <[email protected]>
On Mar 9, 2006, at 8:06 AM, Timothy Legge wrote:
>> -----Original Message-----
>> From: Glenn Brown
>> I'm all for pre-commit reviews, as it's done wonders for our
>> SW development.  At work, we now use this and other parts of
>> the FreeBSD approach, and it works great.
>
> I am certainly fine with this approach.  Is there any guidance on how
> this process will work?

Absolutely!  DWIM "Do what I *mean*" :)

>   Some questions I have are:

Now you want details.  Sheesh. Oh all right.

Ahem.
	
Let me give my preferences in order of easiest question to hardest:

> 3) Should we also attach the patches to Sourceforge to better track
> whether they have been applied?

I think this is a good idea in general, just because things get lost  
sometimes on the mailing list, and they're hard to find.  We can tell  
people that if they don't put it on SF not to complain if it gets  
lost.  Anybody can post on SF.

> 4) Should a third patches mailing list be created?

I'm against it in general because I already think we have too many  
mailing lists :)
Seriously, this fake distinction between "developers" and "users" is  
counterproductive, IMO.
I'm not the baddest hacker in the room, but I'm pretty good, and  
frankly, I  enjoy helping new people.

I like Jim McQuillan's LTSP-Discuss model, and was just about to  
bring up making Etherboot-Discuss, and merge the lists again. Once  
upon a time we were all on the Netboot lists (before SourceForge and  
our split from Netboot), and it was fun.  Sure developers had to  
listen to newbies and newbies had to listen to bit-twiddly stuff, but  
it kept us honest, and sometimes the new people had really good ideas  
and fixed bugs, and it felt less like there were the "super-cool-mega- 
hacker" people and the "supplicant-please-help-me" people.  It kept  
us honest.

But back to your question:

> 4) Should a third patches mailing list be created?

Maybe we should require people to post "long" patches on SourceForge,  
and then send an email letting us know they exist, and send "short"  
stuff as an attachment and put it on SourceForge.  We can control how  
big a message can be posted to the mailing lists, so we can enforce a  
limit.  I don't think we should put too many obstacles in the way of  
the possibility of good code coming in.

> 2) How will the patches get approved?  Should it require comment  
> from a
> few developers?  Which ones?

LOL, Everybody except you. (and Geert)!!! :)

I'm joking.  Actually, I'm only half joking.  In my experience, there  
are a few extremely controversial patches, and mostly the rest are a  
lot of bugfixes. I don't think we need a lot of bureaucracy to handle  
things.  For the easy things, if nobody objects to a patch, I think  
it should generally go in.  If somebody does object, then we should  
talk it over, and if we can't find consensus, I'll make a decision as  
a last resort.

I just don't know how to codify karma and respect, but in 7 years I  
only ran problems a few with controversial issues:

   - PXE_SUFFIX_STRIP option [done]
   - Static IPs in Boot images [working on it]
   - PXE support [done, well, mostly]

As opposed to all the cool stuff I was able to do without a lot of  
problems:

   - Lots of drivers
   - 6 years of rom-o-matic.net
   - Lots of LinuxWorld Expos
   - .zlilo support
   - Making ROMs BBS spec compatible so they show up in boot menus
   - Web page redesign
   - Pretty much anything else I could think of to do

And of the things that were controversial, I have now done or helped  
facilitate most of them.

> 1) Can we commit to branches of the cvs to work on code we would  
> like to
> have in source control prior to submitting it?

Well, we'll have to see how it goes. It is probably alright, if it  
doesn't spoil good conversation.

I think people should say when they are opening a branch and why, in  
general.  Some productive people just feel good in CVS.

But I don't like the idea of too many branches running around, and  
then some hairy huge patch that does 64 things suddenly showing up  
just before we do a release.

I'd much rather see smaller chunks so we can test them, and do more  
incremental releases if necessary.

It also depends on who "we" is.  Some people have enough demonstrated  
ability to be able to get away with it.  Others will have need to  
show something.

> From what I understand, patch management is handled differently by  
> many
> projects.

I'm sure it is.

>   Some require the user to resubmit the patch until it is
> accepted or rejected (lost in email).  Others provide voting on a  
> patch
> or feature.

I believe that.

>   I assume that lack of comment is not implied approval.

Sometimes it is.  Bottom line is that it depends on the situation.

I don't want to bog productive people down with too many constraints,  
but I also don't want people to get lazy and sloppy.

We may have to calibrate this through some practical experience.

> Does someone need to pick up the "trivial" patches that are  
> occasionally
> submitted and submit thim in accordance with the patch process?

Probably.  If someone wants to help out, and this is something they  
want to do, that would be cool, as long as they don't get too big a  
head about it.

I am more concerned about code quality and usefulness than about  
people feeling "special" because they have this priv or that priv.   
Mostly I've found those people can't code and if they can, it isn't  
worth the effort.

I want to keep the rules as simple as will do the job, and leave room  
for people to be creative without being too destructive.

> I know that some of this will work itself out as time goes by but its
> probably a good idea to get everyone on the same page.

Well, sort of on the same page.  I want to keep the overhead low and  
the code flowing without creating artificial barriers.

I agree we need some structure, but I don't want structure just to  
make structure freaks feel safe.  I want it to be a little  
'dangerous' in here -- and by that I mean I want people to feel safe  
expressing themselves, but also being able to deal with the fact that  
they won't always get their way or they might have to defend their  
position a little.

So, that's my take.  Comments and suggestions (or even a good rant)  
are welcome.

I hope this makes things a little clearer for you. :)

> Regards
> Tim

Marty




-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642