[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