Hi!
svn@openbios.org wrote:
Signed-off-by: Adam Kaufman adam.kaufman@pinnacle.com Signed-off-by: Stefan Reinauer stepan@coresystems.de Acked-by: Stefan Reinauer stepan@coresystems.de Acked-by: Adam Kaufman adam.kaufman@pinnacle.com
It seems there is still some confusion about Signed-off-by and Acked-by. The meaning of Acked-by is a real subset of Signed-off-by. So acking a patch you already signed off for is totally meaningless and should be avoided. The Linux kernel guys never ack and sign off the same patch.
Regards, Carl-Daniel
On Tue, Feb 06, 2007 at 10:57:42PM +0100, Carl-Daniel Hailfinger wrote:
Hi!
svn@openbios.org wrote:
Signed-off-by: Adam Kaufman adam.kaufman@pinnacle.com Signed-off-by: Stefan Reinauer stepan@coresystems.de Acked-by: Stefan Reinauer stepan@coresystems.de Acked-by: Adam Kaufman adam.kaufman@pinnacle.com
It seems there is still some confusion about Signed-off-by and Acked-by. The meaning of Acked-by is a real subset of Signed-off-by. So acking a patch you already signed off for is totally meaningless and should be avoided. The Linux kernel guys never ack and sign off the same patch.
I was going to reply to this, too, but for another reason. The last 'Acked-by: Adam Kaufman adam.kaufman@pinnacle.com' was not posted publically by Adam anywhere as far as I can see, so it should not appear in the commit message.
As for the meaning of Acked-by, I'm not so sure. We currently use Signed-off-by as a tagging of "I wrote (parts of) this code" (i.e. is has legal importance), and the Acked-by merely as "ok, looks fine" indicator (but you didn't write the code).
Not sure how exactly the Linux kernel folks use these...
Sample scenario: Developer A posts a patch with a Signed-off-by, you (developer B) modify/improve it and add your own Signed-off-by.
Then someone else (developer C) improves the patch even further, and adds his Signed-off-by. Now you (B) post an Acked-by for this last revision of the patch, i.e.
Signed-off-by: A Signed-off-by: B Signed-off-by: C Acked-by: B
This makes sense to me. What would probably _not_ make any sense is
Signed-off-by: A Signed-off-by: B Acked-by: B
because you ack your own patch here, which defeats our purpose of the Acked-by tag (enforce review by at least one developer).
Also, I don't think that a Signed-off-by necessarily means that you ack the patch. It's possible that you just posted an improved version of the patch (and added your Signed-off-by), but you still think that it's not good enough to be committed, i.e. you don't want to ack it at the moment...
Uwe.
As for the meaning of Acked-by, I'm not so sure. We currently use Signed-off-by as a tagging of "I wrote (parts of) this code" (i.e. is has legal importance), and the Acked-by merely as "ok, looks fine" indicator (but you didn't write the code).
Signed-off-by means "I am (in part) responsible for this ending up in thid repo", i.e., you wrote part of the patch or you were on the path pushing it in.
Not sure how exactly the Linux kernel folks use these...
Acked-by is used as a comment "looks fine by me" when not taking direct action yourself.
Segher
On Fri, Feb 09, 2007 at 06:26:09PM +0100, Segher Boessenkool wrote:
Signed-off-by means "I am (in part) responsible for this ending up in thid repo", i.e., you wrote part of the patch or you were on the path pushing it in.
Acked-by is used as a comment "looks fine by me" when not taking direct action yourself.
Does commiting constitute "on the path pushing it in" ?
Ie. if I review and then commit, should I sign off or ack?
//Peter
Peter Stuge wrote:
On Fri, Feb 09, 2007 at 06:26:09PM +0100, Segher Boessenkool wrote:
Signed-off-by means "I am (in part) responsible for this ending up in thid repo", i.e., you wrote part of the patch or you were on the path pushing it in.
Acked-by is used as a comment "looks fine by me" when not taking direct action yourself.
Does commiting constitute "on the path pushing it in" ?
Ie. if I review and then commit, should I sign off or ack?
Not as a necessary pre-condition.
* If you did not work on the patch, you don't have to sign it off.
* If you don't agree to the patch or have not reviewed it (but check it in because you trust the other reviewers who acked the patch), you should not ack it.
Stefan
Signed-off-by means "I am (in part) responsible for this ending up in thid repo", i.e., you wrote part of the patch or you were on the path pushing it in.
Acked-by is used as a comment "looks fine by me" when not taking direct action yourself.
Does commiting constitute "on the path pushing it in" ?
Yes. Read the DCO if you're still unsure :-)
Ie. if I review and then commit, should I sign off or ack?
Sign off.
Segher
* Segher Boessenkool segher@kernel.crashing.org [070210 20:13]:
Signed-off-by means "I am (in part) responsible for this ending up in thid repo", i.e., you wrote part of the patch or you were on the path pushing it in.
Acked-by is used as a comment "looks fine by me" when not taking direct action yourself.
Does commiting constitute "on the path pushing it in" ?
Yes. Read the DCO if you're still unsure :-)
DCO? Is that an abbreviation for http://www.linuxbios.org/Development_Guidelines?
Ie. if I review and then commit, should I sign off or ack?
Sign off.
I would say ack, but not necessarily sign off.
Thanks for the comments!
On Sat, Feb 10, 2007 at 08:58:23PM +0100, Stefan Reinauer wrote:
Does commiting constitute "on the path pushing it in" ?
Yes. Read the DCO if you're still unsure :-)
DCO? Is that an abbreviation for http://www.linuxbios.org/Development_Guidelines?
DCO is Developer's Certificate of Origin, the blurb that Signed-off-by is shorthand for.
http://osdlab.org/newsroom/press_releases/2004/2004_05_24_dco.html
Speaking of the DCO, we are using the verbatim text of the DCO 1.1 but we have renamed it to "LinuxBIOS Developer's Certificate of Origin 1.1" on the wiki page. Was that intentional?
The original DCO has the following copyright notice: "© 2005 Open Source Development Labs, Inc. The Developer's Certificate of Origin 1.1 is licensed under a Creative Commons Attribution-ShareAlike 2.5 License. If you modify you must use a name or title distinguishable from "Developer's Certificate of Origin" or "DCO" or any confusingly similar name."
I'd like to change the wiki page to make it clear that this is the OSDL DCO and not some local LinuxBIOS variation with a stolen name. Is that OK with everyone?
Ie. if I review and then commit, should I sign off or ack?
Sign off.
I would say ack, but not necessarily sign off.
I guess Segher's point is that committing a patch sent to the mailing list falls under (c) in the DCO, so I should sign off. Is the mailing list really "directly to me" ?
However, I first reviewed the patch, so I should ack it. We want at least one acked-by before commit.
So should I actually first ack and then sign off?
Or do we just agree to roll the two into one for LinuxBIOS? That would make whichever one we choose more ambiguous though. :\
//Peter
Ie. if I review and then commit, should I sign off or ack?
Sign off.
I would say ack, but not necessarily sign off.
If you don't sign off on something, you can't put it into the public tree -- that's the whole philosophy behind the DCO, to have all contributions traceable to their origins, by having a "trail of bread crumbs".
Segher
Ie. if I review and then commit, should I sign off or ack?
Sign off.
I would say ack, but not necessarily sign off.
I guess Segher's point is that committing a patch sent to the mailing list falls under (c) in the DCO, so I should sign off. Is the mailing list really "directly to me" ?
Yes. You got the code, you passed it on. You better make sure that you know what you're signing for though -- i.e., you should make reasonably sure that the person who sent you the patch had the right to do so (whether something is sent via a mailing list makes no difference at all btw -- conducting your business in the open doesn't change the business).
So should I actually first ack and then sign off?
Or do we just agree to roll the two into one for LinuxBIOS? That would make whichever one we choose more ambiguous though. :\
Well it would be really weird to sign-off on a patch that you don't agree with, so acked-by is quite redundant if you already signed off on a patch.
Segher
On Sun, Feb 11, 2007 at 02:42:03AM +0100, Segher Boessenkool wrote:
Ie. if I review and then commit, should I sign off or ack?
Sign off.
I would say ack, but not necessarily sign off.
If you don't sign off on something, you can't put it into the public tree -- that's the whole philosophy behind the DCO, to have all contributions traceable to their origins, by having a "trail of bread crumbs".
Note I did not write the patch and the original author has of course signed off, but is unable to commit herself.
On Sun, Feb 11, 2007 at 02:46:47AM +0100, Segher Boessenkool wrote:
I guess Segher's point is that committing a patch sent to the mailing list falls under (c) in the DCO, so I should sign off. Is the mailing list really "directly to me" ?
Yes. You got the code, you passed it on. You better make sure that you know what you're signing for though -- i.e., you should make reasonably sure that the person who sent you the patch had the right to do so (whether something is sent via a mailing list makes no difference at all btw -- conducting your business in the open doesn't change the business).
Again, the poster has signed off.
So should I actually first ack and then sign off?
Or do we just agree to roll the two into one for LinuxBIOS? That would make whichever one we choose more ambiguous though. :\
Well it would be really weird to sign-off on a patch that you don't agree with, so acked-by is quite redundant if you already signed off on a patch.
I would first review (ack) and then commit (sign off) ..
It seems neither the sign-off nor the ack fits for just a commit.
//Peter
If you don't sign off on something, you can't put it into the public tree -- that's the whole philosophy behind the DCO, to have all contributions traceable to their origins, by having a "trail of bread crumbs".
Note I did not write the patch and the original author has of course signed off, but is unable to commit herself.
[I don't mean you personally of course].
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Yes. You got the code, you passed it on. You better make sure that you know what you're signing for though -- i.e., you should make reasonably sure that the person who sent you the patch had the right to do so (whether something is sent via a mailing list makes no difference at all btw -- conducting your business in the open doesn't change the business).
Again, the poster has signed off.
When you want to pass the code on (for example, by committing it to the repo), you have to sign off on it as well.
Well it would be really weird to sign-off on a patch that you don't agree with, so acked-by is quite redundant if you already signed off on a patch.
I would first review (ack) and then commit (sign off) ..
It seems neither the sign-off nor the ack fits for just a commit.
You *need* a signed-off for a commit though, that's what the DCO is all about.
If what you want is keeping track of committers -- that's not a property of a patch, but a property of the repo; any good SCM tracks that for you automatically.
Segher
On Sun, Feb 11, 2007 at 03:42:43AM +0100, Segher Boessenkool wrote:
If you don't sign off on something, you can't put it into the public tree -- that's the whole philosophy behind the DCO, to have all contributions traceable to their origins, by having a "trail of bread crumbs".
Note I did not write the patch and the original author has of course signed off, but is unable to commit herself.
[I don't mean you personally of course].
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Ok, so our policy is that the committer always adds a sign off?
Again, the poster has signed off.
When you want to pass the code on (for example, by committing it to the repo), you have to sign off on it as well.
But I also reviewed it, so I should ack, right? Adding my own Signed-off-by doesn't imply review, or does it?
It seems neither the sign-off nor the ack fits for just a commit.
You *need* a signed-off for a commit though, that's what the DCO is all about.
Yes, but does the committer need to sign-off too? Isn't it enough with the signed-off-by from the author and an ack from the committer?
If what you want is keeping track of committers -- that's not a property of a patch, but a property of the repo; any good SCM tracks that for you automatically.
Yes.
But the policy of sign-off+ack required for commit is incompatible with the suggested author sign-off+committer sign-off scheme, hence my questions. :)
//Peter
Hi,
On Sat, Feb 10, 2007 at 10:48:33PM +0100, Peter Stuge wrote:
Speaking of the DCO, we are using the verbatim text of the DCO 1.1 but we have renamed it to "LinuxBIOS Developer's Certificate of Origin 1.1" on the wiki page.
Good point.
Was that intentional?
Yes, but that was wrong. I renamed it because I misread the 'If you modify you must use a name or title distinguishable from "Developer's Certificate of Origin"' below. If we do _not_ modify the DCO, we can still also keep the name (that's how I understand it at least).
I reverted my renaming, it's called "Developer's Certificate of Origin 1.1" again now. Thanks for pointing that out.
The original DCO has the following copyright notice: "© 2005 Open Source Development Labs, Inc. The Developer's Certificate of Origin 1.1 is licensed under a Creative Commons Attribution-ShareAlike 2.5 License. If you modify you must use a name or title distinguishable from "Developer's Certificate of Origin" or "DCO" or any confusingly similar name."
Uwe.
Hi,
On Sun, Feb 11, 2007 at 02:46:47AM +0100, Segher Boessenkool wrote:
Well it would be really weird to sign-off on a patch that you don't agree with, so acked-by is quite redundant if you already signed off on a patch.
I posted an example a few days ago. I see a patch on the list, take it, modify it to improve _some_ parts of it, but it's still not "finished" and I'm still don't agree that it should be committed. But at least it's a bit better now.
In such a case I'd say I should sign-off (as I modified the patch), but I don't want to Ack it.
Uwe.
* Uwe Hermann uwe@hermann-uwe.de [070211 16:19]:
I posted an example a few days ago. I see a patch on the list, take it, modify it to improve _some_ parts of it, but it's still not "finished" and I'm still don't agree that it should be committed. But at least it's a bit better now.
In such a case I'd say I should sign-off (as I modified the patch), but I don't want to Ack it.
fully agreed.
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Ok, so our policy is that the committer always adds a sign off?
If not, the whole signed-off-by thing becomes useless, so it better be policy.
When you want to pass the code on (for example, by committing it to the repo), you have to sign off on it as well.
But I also reviewed it, so I should ack, right?
Dunno. "acked-by" as used in Linux is only an informal comment; if LinuxBIOS wants to formalise its usage, the rules should be written down somewhere.
Adding my own Signed-off-by doesn't imply review, or does it?
It doesn't, but it would be silly (and irresponsible) to sign-off on something you didn't look at first.
It seems neither the sign-off nor the ack fits for just a commit.
You *need* a signed-off for a commit though, that's what the DCO is all about.
Yes, but does the committer need to sign-off too? Isn't it enough with the signed-off-by from the author and an ack from the committer?
No. Every step in the chain into the repo needs to be tracked or the "chain of trust" is lost.
If what you want is keeping track of committers -- that's not a property of a patch, but a property of the repo; any good SCM tracks that for you automatically.
Yes.
But the policy of sign-off+ack required for commit is incompatible with the suggested author sign-off+committer sign-off scheme, hence my questions. :)
I don't see the incompatibility? Unless you mean that the acked-by tags should be put into the commit; that is a foolish thing indeed, there are many problems with it (for example, it is easy to forget to add one of those when you commit; not the case with signed-off, since that's in the patch when you send it out for review already, and a committer will add it automatically if he has his tools set up for that).
Segher
Well it would be really weird to sign-off on a patch that you don't agree with, so acked-by is quite redundant if you already signed off on a patch.
I posted an example a few days ago. I see a patch on the list, take it, modify it to improve _some_ parts of it, but it's still not "finished" and I'm still don't agree that it should be committed. But at least it's a bit better now.
In such a case I'd say I should sign-off (as I modified the patch), but I don't want to Ack it.
But you don't check it in yet either. The "ack" that we require is only for pushing stuff into the repo.
Segher
I posted an example a few days ago. I see a patch on the list, take it, modify it to improve _some_ parts of it, but it's still not "finished" and I'm still don't agree that it should be committed. But at least it's a bit better now.
In such a case I'd say I should sign-off (as I modified the patch), but I don't want to Ack it.
fully agreed.
Yeah. But the only "acked-by"s that have any sense tracking are those on the final version of the patch, i.e. the version that gets checked in, so the point is moot.
Segher
* Segher Boessenkool segher@kernel.crashing.org [070212 00:49]:
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Ok, so our policy is that the committer always adds a sign off?
If not, the whole signed-off-by thing becomes useless, so it better be policy.
now, why exactly?
But I also reviewed it, so I should ack, right?
Dunno. "acked-by" as used in Linux is only an informal comment; if LinuxBIOS wants to formalise its usage, the rules should be written down somewhere.
Whats missing in http://www.linuxbios.org/Development_Guidelines?
Yes, but does the committer need to sign-off too? Isn't it enough with the signed-off-by from the author and an ack from the committer?
No. Every step in the chain into the repo needs to be tracked or the "chain of trust" is lost.
I dont think the chain of trust goes lost. The repository monitors who did the commit, so it will be as easy to find out as grepping for the Signed-off-by: ?
ie. Are you saying the mails that get sent out to the mailing list should be filtered to say
Signed-off-by: Committer
instead of
Committed by: Committer
?
I don't see the incompatibility? Unless you mean that the acked-by tags should be put into the commit; that is a foolish thing indeed, there are many problems with it (for example, it is easy to forget to add one of those when you commit; not the case with signed-off, since that's in the patch when you send it out for review already, and a committer will add it automatically if he has his tools set up for that).
If you think our review process is useless, you are of course not forced to contribute to it.
Stefan
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Ok, so our policy is that the committer always adds a sign off?
If not, the whole signed-off-by thing becomes useless, so it better be policy.
now, why exactly?
It's point (c) in the DCO.
If you allow any code to be checked in without the person doing that stating he has the the right to do that, i.e. without adding the signed-off, all previous signed-off statements (by the original developer, etc.) have no significance as to whether the LinuxBIOS project did check if it was allowed (for IP or copyright reasons) to use the code. Only full chains work; one missing link and it's broken.
But I also reviewed it, so I should ack, right?
Dunno. "acked-by" as used in Linux is only an informal comment; if LinuxBIOS wants to formalise its usage, the rules should be written down somewhere.
Whats missing in http://www.linuxbios.org/Development_Guidelines?
The doc should be in the repo itself. Other than that, it could be formalised a bit ;-)
Yes, but does the committer need to sign-off too? Isn't it enough with the signed-off-by from the author and an ack from the committer?
No. Every step in the chain into the repo needs to be tracked or the "chain of trust" is lost.
I dont think the chain of trust goes lost. The repository monitors who did the commit, so it will be as easy to find out as grepping for the Signed-off-by: ?
The "commit" message the repo gives you only tells you who did the check in; it doesn't say that the commiter states that he checked to the best of his knowledge that he is allowed to (re-)publish that source code.
ie. Are you saying the mails that get sent out to the mailing list should be filtered to say
Signed-off-by: Committerinstead of
Committed by: Committer?
No. I'm saying that committers should manually (or at least consciously) add the signed-off.
I don't see the incompatibility? Unless you mean that the acked-by tags should be put into the commit; that is a foolish thing indeed, there are many problems with it (for example, it is easy to forget to add one of those when you commit; not the case with signed-off, since that's in the patch when you send it out for review already, and a committer will add it automatically if he has his tools set up for that).
If you think our review process is useless, you are of course not forced to contribute to it.
I'm not saying the review process is useless; I'm saying that recording history of who thought what patch was a good idea, _when those patches never end up being committed_, is pretty damn useless. A newer version of the patch superseded the old one; knowing who approved the final commit *can* of course be useful. I wasn't commenting on the review process at all; just on the acked-by lines that people add to commit messages.
Segher
Hi,
On Mon, Feb 12, 2007 at 03:20:45PM +0100, Segher Boessenkool wrote:
It's point (c) in the DCO.
If you allow any code to be checked in without the person doing that stating he has the the right to do that, i.e. without adding the signed-off, all previous signed-off statements (by the original developer, etc.) have no significance as to whether the LinuxBIOS project did check if it was allowed (for IP or copyright reasons) to use the code. Only full chains work; one missing link and it's broken.
Hm, good point.
But I also reviewed it, so I should ack, right?
Dunno. "acked-by" as used in Linux is only an informal comment; if LinuxBIOS wants to formalise its usage, the rules should be written down somewhere.
Whats missing in http://www.linuxbios.org/Development_Guidelines?
The doc should be in the repo itself.
OK, I'll send a patch soon.
Other than that, it could be formalised a bit ;-)
Do you have a patch or specific suggestions for improvements?
I'm not saying the review process is useless; I'm saying that recording history of who thought what patch was a good idea, _when those patches never end up being committed_, is pretty damn useless. A newer version of the patch superseded the old one; knowing who approved the final commit *can* of course be useful. I wasn't commenting on the review process at all; just on the acked-by lines that people add to commit messages.
OK, how about this procedure (I don't really care anymore whether it's compatible with the way it works in Linux, it should only be legally "bullet-proof"):
* Everyone who creates or modifies a patch adds his Signed-off-by.
* The person who finally commits the patch adds his/her Signed-off-by, too (if it's not already there anyway).
* The Acked-by is completely separated from that. You send an Acked-by when you think this patch can be committed. You don't have to modify a patch for an Acked-by, you can just send it to say "I think this patch is ok".
* If a certain version of a patch received two Acked-by's by two different people, it can be committed. Ergo, every commit message will have 1 or more Signed-off-by lines which build a "chain of trust" for legal reasons, _and_ it will have 2 or more Acked-by lines which enforce our review process.
* The Acked-by's must be for exactly the same version of the patch. Acked-by's for previous versions of the patch are meaningless, they are not added to the commit message, only those for the exact incarnation of the patch which gets committed.
* So yes, it is possible to post
- A patch with only a Sign-off-by: You modified the code, but don't want it to be committed, yet. - A patch with a Signed-off-by and an Acked-by: You modified the patch and you think it can be commited. - An email with just an Acked-by: You didn't touch the patch at all, but you think it can be committed.
Comments?
Uwe.
Hi,
On Tue, Feb 13, 2007 at 01:15:09PM +0100, Uwe Hermann wrote:
OK, how about this procedure (I don't really care anymore whether it's compatible with the way it works in Linux, it should only be legally "bullet-proof"):
Everyone who creates or modifies a patch adds his Signed-off-by.
The person who finally commits the patch adds his/her Signed-off-by, too (if it's not already there anyway).
The Acked-by is completely separated from that. You send an Acked-by when you think this patch can be committed. You don't have to modify a patch for an Acked-by, you can just send it to say "I think this patch is ok".
If a certain version of a patch received two Acked-by's by two different people, it can be committed. Ergo, every commit message will have 1 or more Signed-off-by lines which build a "chain of trust" for legal reasons, _and_ it will have 2 or more Acked-by lines which enforce our review process.
The Acked-by's must be for exactly the same version of the patch. Acked-by's for previous versions of the patch are meaningless, they are not added to the commit message, only those for the exact incarnation of the patch which gets committed.
So yes, it is possible to post
- A patch with only a Sign-off-by: You modified the code, but don't want it to be committed, yet.
- A patch with a Signed-off-by and an Acked-by: You modified the patch and you think it can be commited.
- An email with just an Acked-by: You didn't touch the patch at all, but you think it can be committed.
No comments, no objections? Shall I update the wiki with this procedure and shall we use it from now on?
I.e., you sign-off everything you touch or apply, you can ack your own patches, and any commit must get at least to acks (e.g. yours and that of one further developer).
Uwe.
On 2/22/07, Uwe Hermann uwe@hermann-uwe.de wrote:
Hi,
On Tue, Feb 13, 2007 at 01:15:09PM +0100, Uwe Hermann wrote:
OK, how about this procedure (I don't really care anymore whether it's compatible with the way it works in Linux, it should only be legally "bullet-proof"):
Everyone who creates or modifies a patch adds his Signed-off-by.
The person who finally commits the patch adds his/her Signed-off-by, too (if it's not already there anyway).
And puts in the commit line from svn, e.g. Commited revision 204
The Acked-by is completely separated from that. You send an Acked-by when you think this patch can be committed. You don't have to modify a patch for an Acked-by, you can just send it to say "I think this patch is ok".
If a certain version of a patch received two Acked-by's by two different people, it can be committed. Ergo, every commit message will have 1 or more Signed-off-by lines which build a "chain of trust" for legal reasons, _and_ it will have 2 or more Acked-by lines which enforce our review process.
The Acked-by's must be for exactly the same version of the patch. Acked-by's for previous versions of the patch are meaningless, they are not added to the commit message, only those for the exact incarnation of the patch which gets committed.
So yes, it is possible to post
- A patch with only a Sign-off-by: You modified the code, but don't want it to be committed, yet.
- A patch with a Signed-off-by and an Acked-by: You modified the patch and you think it can be commited.
- An email with just an Acked-by: You didn't touch the patch at all, but you think it can be committed.
No comments, no objections? Shall I update the wiki with this procedure and shall we use it from now on?
I.e., you sign-off everything you touch or apply, you can ack your own patches, and any commit must get at least to acks (e.g. yours and that of one further developer).
I think I understand this now, and it is ok by me, if the line Commit appears in a message which is telling us a commit happened. I think it is important that we know if a patch has been committed. There have been some big patches lately that were in an undertermined state because they got signed off, and acked, and never committed, and i could not tell what had happened.
So if you have a thread, and you see signed-off and acked lines, but no commit lines, you can assume the patch was not committed. Right now, you just can not tell.
So if you signed off a patch, you are also going to ack it in the same email in most cases; this seems a little weird to me, I just assumed signed-off-by could apply acked-by, but I guess not?
thanks
ron
Whats missing in http://www.linuxbios.org/Development_Guidelines?
The doc should be in the repo itself.
OK, I'll send a patch soon.
Great, thanks.
Other than that, it could be formalised a bit ;-)
Do you have a patch or specific suggestions for improvements?
Nope, I can't say I'm good at writing pseudo-legalese myself.
I'm not saying the review process is useless; I'm saying that recording history of who thought what patch was a good idea, _when those patches never end up being committed_, is pretty damn useless. A newer version of the patch superseded the old one; knowing who approved the final commit *can* of course be useful. I wasn't commenting on the review process at all; just on the acked-by lines that people add to commit messages.
OK, how about this procedure (I don't really care anymore whether it's compatible with the way it works in Linux, it should only be legally "bullet-proof"):
Everyone who creates or modifies a patch adds his Signed-off-by.
The person who finally commits the patch adds his/her Signed-off-by, too (if it's not already there anyway).
In the Linux logs, there's also an "Author:" field next to the patch subjects and date; normally it's automatically picked up from the "From:" line on the email patch (or from an explicit "From:" line in the patch). LinuxBIOS might want to have this author information more explicit, too (yes almost always the author is the same as the first signed-off-by, but there can be multiple authors for a patch, or the sign-off is done by someone else than the author, someone from the same company for example).
- The Acked-by is completely separated from that.
Exactly.
You send an Acked-by when you think this patch can be committed. You don't have to modify a patch for an Acked-by, you can just send it to say "I think this patch is ok".
- If a certain version of a patch received two Acked-by's by two different people, it can be committed. Ergo, every commit message will have 1 or more Signed-off-by lines
which build a "chain of trust" for legal reasons, _and_ it will have 2 or more Acked-by lines which enforce our review process.
- The Acked-by's must be for exactly the same version of the patch. Acked-by's for previous versions of the patch are meaningless, they are not added to the commit message, only those for the exact incarnation of the patch which gets committed.
Sounds all fine to me.
So yes, it is possible to post
- A patch with only a Sign-off-by: You modified the code, but don't want it to be committed, yet.
You better state that "it's not ready yet" explicitly too in such a case, to avoid confusion.
- A patch with a Signed-off-by and an Acked-by: You modified the patch and you think it can be commited.
That's the normal case. I think ack'ing your own patches is pretty meaningless (you cannot really review your own patches) but maybe that's just me.
- An email with just an Acked-by: You didn't touch the patch at all, but you think it can be
committed.
If your ACK is the last needed one, you can just go ahead and commit the patch and send an email stating you did that. The eventual committer has to type in the acked-by lines anyway.
Comments?
Just one about process: it is good practice to keep the signed-off-by in the patch itself, so you won't mess-em up. Patch maintenance tools like quilt can help here.
Segher
No comments, no objections?
I didn't see your mail until just now, together with this one.
Shall I update the wiki with this procedure and shall we use it from now on?
I.e., you sign-off everything you touch or apply, you can ack your own patches, and any commit must get at least to acks (e.g. yours and that of one further developer).
Well I don't see the point of two acks if one is your own -- if you don't want your patch committed yet, just say so (post it as an RFC). Just have the rule be simply that there needs to be an ack from a committer and that's it.
Segher
I think I understand this now, and it is ok by me, if the line Commit appears in a message which is telling us a commit happened. I think it is important that we know if a patch has been committed.
I tend to get the mail from the SVN daemon earlier than the one from the discussion list, but yeah, it's good to end a mail thread with "committed as rNNN".
There have been some big patches lately that were in an undertermined state because they got signed off, and acked, and never committed, and i could not tell what had happened.
The way it's done on most GNU projects (that I know off anyway), is that patches from "outsiders" get committed by whomever acks it (and you don't need to do a separate mail saying "ack", you just reply "thanks, commited"), and patches from "insiders" are normally committed by the submitter of the patch. LinuxBIOS might want to do something similar.
So if you have a thread, and you see signed-off and acked lines, but no commit lines, you can assume the patch was not committed. Right now, you just can not tell.
Yeah.
So if you signed off a patch, you are also going to ack it in the same email in most cases; this seems a little weird to me, I just assumed signed-off-by could apply acked-by, but I guess not?
They are separate things. I still don't see the point of having the acked-by in the commit message; we don't need to track this for a legal or similar reason, it's really easy to get wrong or forget (the committer has to do it as the last step before commit, there is no review possibility), and for the typical use of this tag ("who approved this patch?") you want to see the whole email thread anyway. It also doesn't say "this patch was correctly reviewed": a) the committer puts in the acked-by lines himself; b) whoever committed it implicitly acked the patch. The only case where this doesn't hold is if you don't trust the people who have commit access to the repo (to a certain level at least), and then you're hosed anyway ;-)
Segher
On 22.02.2007 18:39, Segher Boessenkool wrote:
So yes, it is possible to post
- A patch with only a Sign-off-by: You modified the code, but don't want it to be committed, yet.
You better state that "it's not ready yet" explicitly too in such a case, to avoid confusion.
Very much agreed. A "don't commit" or "RFC" message is much more explicit than a missing ack.
- A patch with a Signed-off-by and an Acked-by: You modified the patch and you think it can be commited.
That's the normal case. I think ack'ing your own patches is pretty meaningless (you cannot really review your own patches) but maybe that's just me.
Yes. If you don't want to ack a patch, simply post it as RFC. As Segher said: You can't really review your own patches.
- An email with just an Acked-by: You didn't touch the patch at all, but you think it can be
committed.
If your ACK is the last needed one, you can just go ahead and commit the patch and send an email stating you did that. The eventual committer has to type in the acked-by lines anyway.
There's one exception: If you don't have commit rights (yet), but you want to ack a patch.
So except for the acking I agree with Uwe.
Regards, Carl-Daniel
On Mon, Feb 12, 2007 at 03:20:45PM +0100, Segher Boessenkool wrote:
You can only commit a patch to the tree if you take responsibility for it (at some level), and that means you'll have to sign off on it.
Ok, so our policy is that the committer always adds a sign off?
If not, the whole signed-off-by thing becomes useless, so it better be policy.
now, why exactly?
It's point (c) in the DCO.
If you allow any code to be checked in without the person doing that stating he has the the right to do that, i.e. without adding the signed-off, all previous signed-off statements (by the original developer, etc.) have no significance as to whether the LinuxBIOS project did check if it was allowed (for IP or copyright reasons) to use the code. Only full chains work; one missing link and it's broken.
This means that the practice of adding an ack while committing a patch that you did not take part in writing is incorrect.
The "commit" message the repo gives you only tells you who did the check in; it doesn't say that the commiter states that he checked to the best of his knowledge that he is allowed to (re-)publish that source code.
The correct action would be to sign off. But then there is no ack (only multiple sign offs) and so not at least one developer has acked the patch.
This is inconsistent and confusing. We just need to settle on one way to do it and document that.
//Peter
-
Carl-Daniel Hailfinger -
Peter Stuge -
ron minnich -
Segher Boessenkool -
Stefan Reinauer -
Stefan Reinauer -
Uwe Hermann