[LinuxBIOS] Patch quality

Corey Osgood corey.osgood at gmail.com
Tue Oct 30 02:54:15 CET 2007


Stefan Reinauer wrote:
> * ron minnich <rminnich at gmail.com> [071029 23:51]:
>   
>> One possibility is, the next time someone comes in, rather than just
>> critiquing the code, we get a group of people -- say 3 or 4 -- each of
>> whom takes on the responsibility for a directory, starting at the
>> leaves. Something like this:
>>
>> superio (s)
>> southbridge(s)
>> northbridges(s)
>> cpu
>>     
>  
> In addition: New components are undangerous to add as long as they are
> signed off and do not contain proprietary licensed code. We can just
> commit them before finding a shepherd - Nothing will break except the
> fresh code is maybe not working yet.
>   

Completely agree!!!

> The advantage is clear: Instead of keeping iterations of work on the
> vendor side while we waste our time on writing mails, we can spend the
> same time fixing the code ourselfes and take that burdon off the
> committers. Repeatedly asking for patches because of missing full stops
> and unsigned char -> uint8_t conversions is more effort than producing a
> stressful interface to potential committers. We need to become easier to
> handle in this way.
>   

Again, agreed. If this were the linux kernel, the patches might be
outright rejected over things like this. OTOH, we aren't the kernel, we
don't have nearly the status nor the manpower.

>> once those are absorbed, we do the mainboard, and then, the target.
>>
>> This is the 'shepherd' model.
>>     
>
> I like this idea a lot. The shepherds are the extended and more
> work-directed versions of reviewers. (which should not go away)
>
>   
>> This team would form dynamically and communicate (via the list?) on
>> the changes that have to be made. But people would have to sign up for
>> a given piece, and a deadline.
>>     
>  
> How would signing up work? We have a pretty active community with many
> people having something to contribute. Assigning tasks and duties and
> deadlines might very well scare people off that would have something
> valuable to say, otherwise. Therefore I believe we should handle this
> very casual.

I think we have some lurkers on the list who might be willing to step up
and handle some of this ;) I know I barely have time to clean up my own
code, let alone others.

>> But if we can't do better when someone like Morgan comes in, it's
>> going to hurt us. I don't feel (me included) that we did what we
>> needed to do. The 'code critique' approach works in some cases, but
>> for those who need more assistance, we need a different procedure,
>> which would be more active and helpful.
>>     
>
> I feel we became far to strict with the requirements to the contributors
> lately. 
>   

I completely agree. However, I don't think requiring code in the form of
a patch is too strict a requirement. But these endless reviews over
trivial whitespace fixes, etc, get very annoying and very frustrating
fast. These can be cleaned up later in the form of (much smaller!) patches.

> 1. We do not want to educate anyone that does not ask for it
> 2. We do want to get as many contributions as possible, therefore it
>    should be as easy as possible.
> 3. We need to beware of legal issues
> 4. We aim at having code of the best possible quality.
> 5. We do not want to break the tree.
>
> Code reviews must protect us from issues with 3 and 5.
>
> Shepherdship shall help us with 2 and 4.
>
> Can we constitute this in the development guidelines?
>   

Sounds like a good idea to me.

-Corey




More information about the coreboot mailing list