[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.


More information about the coreboot mailing list