[LinuxBIOS] Purpose of Acked-by

Segher Boessenkool segher at kernel.crashing.org
Mon Dec 11 17:13:43 CET 2006


>> Acked-by is just a comment saying who approved this going into the  
>> SVN
>> tree, it is completely separate; it should probably be called
>> Approved-by or something like that.  I don't really see it having any
>> real purpose, but maybe that's just me :-)
>
> In firmware development the risk of bricking your machine is very  
> high,
> even with small changes that might have a side effect that the  
> original
> author does not see or does not care about. We have had this case so
> many times and I've been spending many days fixing the tree after
> someone broke it like that and just stopped submitting patches after
> that.
>
> On the other hand, pointing fingers within a team is useless, it wont
> make teamwork any better. Thus we agreed on using a "second set of  
> eyes"
> principle for all work, and expressing this by the Acked-by tag.

Oh, I fully understand why patches should be reviewed by
other people, and why someone "senior" should approve
patches before they are committed to the SVN tree.

I'm just challenging the usefulness of the Acked-by tag
in the commit message.

> I understand that amongs developers such a principle of control is
> observed distrustfully, or considered free of purpose, but in fact  
> it is
> not.

I don't disagree.

> Now the habit of Acking your own patches is a well-known workaround to
> this 2 eyes principle. It is _supposed_ to look stupid so that people
> normally dont do it, unless they fulfilled a couple of criteria
> 1) they waited for a review for an appropriate amount of time

This is failing recently in my observation.

> 2) they ran abuild

As soon as abuild works for everyone, this should be made
a requirement.  Even now already, we should encourage people
to say with their patch submissions "abuild ran with no new
failures" or similar.

> 3) they are absolutely certain that the change is so trivial
>    that it wont break anything

Yeah right :-)

> 4) they are absolutely certain that the change is so trivial
>    that noone would ever oppose.

Heh right :-)

There are a 5) and 6) also:

5) Emergency fixes.  Hopefully never needed and better discussed
widely before committing.  This should really just be treated as
an exception outside of the normal framework I guess.

6) Revert of one's own recent commits, if problems show up after
the fact.

> If people start misusing this method, we might just remove them  
> from the
> list of committers.

Of course.  Misuse is a separate problem :-)


Segher





More information about the coreboot mailing list