Re: RFC: reviewer's statement of oversight
On Mon, Oct 15, 2007 at 10:35:39AM +1000, Neil Brown wrote: > On Tuesday October 9, [EMAIL PROTECTED] wrote: > > Hi Neil. > > > > > >From:The Author, Primary Author, or Authors of the patch. > > > Authors should also provide a Signed-off-by: tag. > > > > > > Purpose: to give credit to authors > > The SCM should include this info and we should not duplicate this > > in the changelog's. > > I know some tools require this format but that's something else. > > If the SCM stores some tags in special places, that is fine with me. > The remove the need for the tag and an understanding of why it exists. > Can 'git' store a list of Authors? Do we want to allow a list? git stores to my best knowledge only a single author. Infrequently we need a list but then people have solved it putting relevant people in s-o-b and by give credit in changelog. This is IMHO good enugh. > > > > > > > + > > > > +Signed-off-by: A person adding a Signed-off-by tag is attesting that > > > > the > > > > + patch is, to the best of his or her knowledge, legally > > > > able > > > > + to be merged into the mainline and distributed under the > > > > + terms of the GNU General Public License, version 2. See > > > > + the Developer's Certificate of Origin, found in > > > > + Documentation/SubmittingPatches, for the precise > > > > meaning of > > > > + Signed-off-by. > > > > > > Purpose: to allow subsequent review of the originality of > > > the contribution should copyright questions arise. > > > > We often use s-o-b to docuemnt the path a patch took from origin (the > > top-most s-o-b) to tree apply (lowest s-o-b). > > This is IIUC part of the intended behaviour of s-o-b but it is not > > clear from the above text. > > My understanding of Andrew Morton's position on s-o-b is that it is an > unordered set. I know this because when I have sent him patches with > a proper From: line, he has complained and begrudingly took the first > s-o-b, but said he didn't like to. > So there seems to be disagreement on this (I think it looks like a > path to - but apparently not to everyone). With the current definition you need to supply BOTH a from: and a s-o-b. I usually request a s-o-b when it is missing no matter what other content is present in the changelog. > > > > > > > > > + > > > > +Acked-by: The person named (who should be an active developer in > > > > the > > > > + area addressed by the patch) is aware of the patch and > > > > has > > > > + no objection to its inclusion. An Acked-by tag does not > > > > + imply any involvement in the development of the patch or > > > > + that a detailed review was done. > > > > > > Purpose: to inform upstream aggregators that > > > consensus was achieved for the change. This is > > > particularly relevant for changes that affect multiple > > > Maintenance Domains. > > > > > consensus seems too strong a wording here. consensus imply more than one > > that agree on the patch where I often see people give their "Acked-by:" by > > simple changelog reading. > > I'm failing to follow your logic. > You seem to be contrasting: > "consensus imply more than one that agree" > which I agree with: "From" plus all "Acked-By" will be more than > one in all cases that "Acked-By" is used I did not realise that "concensus" in this context refered to both the one that give the "Acked-by" and the author. Viewing it this way I agree with the intent and the text. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 15, 2007 at 10:35:39AM +1000, Neil Brown wrote: On Tuesday October 9, [EMAIL PROTECTED] wrote: Hi Neil. From:The Author, Primary Author, or Authors of the patch. Authors should also provide a Signed-off-by: tag. Purpose: to give credit to authors The SCM should include this info and we should not duplicate this in the changelog's. I know some tools require this format but that's something else. If the SCM stores some tags in special places, that is fine with me. The remove the need for the tag and an understanding of why it exists. Can 'git' store a list of Authors? Do we want to allow a list? git stores to my best knowledge only a single author. Infrequently we need a list but then people have solved it putting relevant people in s-o-b and by give credit in changelog. This is IMHO good enugh. + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. Purpose: to allow subsequent review of the originality of the contribution should copyright questions arise. We often use s-o-b to docuemnt the path a patch took from origin (the top-most s-o-b) to tree apply (lowest s-o-b). This is IIUC part of the intended behaviour of s-o-b but it is not clear from the above text. My understanding of Andrew Morton's position on s-o-b is that it is an unordered set. I know this because when I have sent him patches with a proper From: line, he has complained and begrudingly took the first s-o-b, but said he didn't like to. So there seems to be disagreement on this (I think it looks like a path to - but apparently not to everyone). With the current definition you need to supply BOTH a from: and a s-o-b. I usually request a s-o-b when it is missing no matter what other content is present in the changelog. + +Acked-by: The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. Purpose: to inform upstream aggregators that consensus was achieved for the change. This is particularly relevant for changes that affect multiple Maintenance Domains. consensus seems too strong a wording here. consensus imply more than one that agree on the patch where I often see people give their Acked-by: by simple changelog reading. I'm failing to follow your logic. You seem to be contrasting: consensus imply more than one that agree which I agree with: From plus all Acked-By will be more than one in all cases that Acked-By is used I did not realise that concensus in this context refered to both the one that give the Acked-by and the author. Viewing it this way I agree with the intent and the text. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tuesday October 9, [EMAIL PROTECTED] wrote: > Hi Neil. > > > >From:The Author, Primary Author, or Authors of the patch. > > Authors should also provide a Signed-off-by: tag. > > > > Purpose: to give credit to authors > The SCM should include this info and we should not duplicate this > in the changelog's. > I know some tools require this format but that's something else. If the SCM stores some tags in special places, that is fine with me. The remove the need for the tag and an understanding of why it exists. Can 'git' store a list of Authors? Do we want to allow a list? > > > > + > > > +Signed-off-by: A person adding a Signed-off-by tag is attesting that the > > > + patch is, to the best of his or her knowledge, legally able > > > + to be merged into the mainline and distributed under the > > > + terms of the GNU General Public License, version 2. See > > > + the Developer's Certificate of Origin, found in > > > + Documentation/SubmittingPatches, for the precise meaning of > > > + Signed-off-by. > > > > Purpose: to allow subsequent review of the originality of > > the contribution should copyright questions arise. > > We often use s-o-b to docuemnt the path a patch took from origin (the > top-most s-o-b) to tree apply (lowest s-o-b). > This is IIUC part of the intended behaviour of s-o-b but it is not > clear from the above text. My understanding of Andrew Morton's position on s-o-b is that it is an unordered set. I know this because when I have sent him patches with a proper From: line, he has complained and begrudingly took the first s-o-b, but said he didn't like to. So there seems to be disagreement on this (I think it looks like a path to - but apparently not to everyone). > > > > > + > > > +Acked-by:The person named (who should be an active developer in > > > the > > > + area addressed by the patch) is aware of the patch and has > > > + no objection to its inclusion. An Acked-by tag does not > > > + imply any involvement in the development of the patch or > > > + that a detailed review was done. > > > > Purpose: to inform upstream aggregators that > > consensus was achieved for the change. This is > > particularly relevant for changes that affect multiple > > Maintenance Domains. > > > consensus seems too strong a wording here. consensus imply more than one > that agree on the patch where I often see people give their "Acked-by:" by > simple changelog reading. I'm failing to follow your logic. You seem to be contrasting: "consensus imply more than one that agree" which I agree with: "From" plus all "Acked-By" will be more than one in all cases that "Acked-By" is used with "people give their "Acked-by:" by simple changlog reading" which I also agree with but this just highlights that "Acked-by" is different from "Reviewed-by" Confused. Thanks, NeilBrown - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Wednesday October 10, [EMAIL PROTECTED] wrote: > On Tue, Oct 09, 2007 at 10:49:20AM -0600, Jonathan Corbet wrote: > > Neil Brown <[EMAIL PROTECTED]> wrote: > > > > + (b) Any problems, concerns, or questions relating to the patch have > > > > been > > > > + communicated back to the submitter. I am satisfied with how the > > > > + submitter has responded to my comments. > > > > > > This seems more detailed that necessary. The process (communicated > > > back / responded) is not really relevant. > > > > Instead, it seems to me that the process is crucially important. > > Reviewed-by shouldn't be a rubber stamp that somebody applies to a > > patch; I think it should really imply that issues of interest have been > > communicated to the developers. If we are setting expectations for what > > Reviewed-by means, I would prefer to leave an explicit mention of > > communication in there. > > I couldn't agree more, Jon. > > If we are to have a meaningful reviewed-by tag, it has to be clearly > documented as to what responsibilities it places on the reviewer. If > someone doesn't want to perform a well conducted review, then they > haven't earned the right to issue a Reviewed-by tag - they can use > the Acked-by rubber stamp instead. Maybe I'm making a mountain out of a molehill but... Clearly documented responsibilities? Yes. Prescribed process? No. If someone sends me a patch, and I review it, and I find a couple of problems, do I need to negotiate with the submitter before correcting them and putting a "Reviewed-by" tag on it (along with my Signed-off-by before sending it upstream)? The above clause (b) seems to say that I do. Is that something we want to mandate? My take on the responsibilities implied by Reviewed-by: is that the code has been inspected, comprehended, considered, and found to be both appropriate and without discernible error. The process by which the code got to that state is not relevant to the tag (though it probably is relevant to the general health of the community). NeilBrown - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Wednesday October 10, [EMAIL PROTECTED] wrote: On Tue, Oct 09, 2007 at 10:49:20AM -0600, Jonathan Corbet wrote: Neil Brown [EMAIL PROTECTED] wrote: + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. Instead, it seems to me that the process is crucially important. Reviewed-by shouldn't be a rubber stamp that somebody applies to a patch; I think it should really imply that issues of interest have been communicated to the developers. If we are setting expectations for what Reviewed-by means, I would prefer to leave an explicit mention of communication in there. I couldn't agree more, Jon. If we are to have a meaningful reviewed-by tag, it has to be clearly documented as to what responsibilities it places on the reviewer. If someone doesn't want to perform a well conducted review, then they haven't earned the right to issue a Reviewed-by tag - they can use the Acked-by rubber stamp instead. Maybe I'm making a mountain out of a molehill but... Clearly documented responsibilities? Yes. Prescribed process? No. If someone sends me a patch, and I review it, and I find a couple of problems, do I need to negotiate with the submitter before correcting them and putting a Reviewed-by tag on it (along with my Signed-off-by before sending it upstream)? The above clause (b) seems to say that I do. Is that something we want to mandate? My take on the responsibilities implied by Reviewed-by: is that the code has been inspected, comprehended, considered, and found to be both appropriate and without discernible error. The process by which the code got to that state is not relevant to the tag (though it probably is relevant to the general health of the community). NeilBrown - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tuesday October 9, [EMAIL PROTECTED] wrote: Hi Neil. From:The Author, Primary Author, or Authors of the patch. Authors should also provide a Signed-off-by: tag. Purpose: to give credit to authors The SCM should include this info and we should not duplicate this in the changelog's. I know some tools require this format but that's something else. If the SCM stores some tags in special places, that is fine with me. The remove the need for the tag and an understanding of why it exists. Can 'git' store a list of Authors? Do we want to allow a list? + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. Purpose: to allow subsequent review of the originality of the contribution should copyright questions arise. We often use s-o-b to docuemnt the path a patch took from origin (the top-most s-o-b) to tree apply (lowest s-o-b). This is IIUC part of the intended behaviour of s-o-b but it is not clear from the above text. My understanding of Andrew Morton's position on s-o-b is that it is an unordered set. I know this because when I have sent him patches with a proper From: line, he has complained and begrudingly took the first s-o-b, but said he didn't like to. So there seems to be disagreement on this (I think it looks like a path to - but apparently not to everyone). + +Acked-by:The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. Purpose: to inform upstream aggregators that consensus was achieved for the change. This is particularly relevant for changes that affect multiple Maintenance Domains. consensus seems too strong a wording here. consensus imply more than one that agree on the patch where I often see people give their Acked-by: by simple changelog reading. I'm failing to follow your logic. You seem to be contrasting: consensus imply more than one that agree which I agree with: From plus all Acked-By will be more than one in all cases that Acked-By is used with people give their Acked-by: by simple changlog reading which I also agree with but this just highlights that Acked-by is different from Reviewed-by Confused. Thanks, NeilBrown - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, Jonathan Corbet <[EMAIL PROTECTED]> wrote: Some minor rewording suggestions: > + (b) Any problems, concerns, or questions relating to the patch have been > + communicated back to the submitter. I am satisfied with how the > + submitter has responded to my comments. --- Replace the last sentence with "I am satisfied with the submitter's response to my comments." or "The submitter has responded to my comments in a way that satisfied my concerns." --- > + > + (c) While there may (or may not) be things which could be improved with > + this submission, I believe that it is, at this time, (1) a worthwhile > + modification to the kernel, and (2) free of known issues which would > + argue against its inclusion. --- I would suggest dropping the "(or may not)" as unnecessary, and changing the "which would" to "that would". --- > + > + (d) While I have reviewed the patch and believe it to be sound, I can not --- >From a legal standpoint, "I do not" might be preferable to "I cannot", since it disclaims any intention to make such a statement, regardless of qualification. --- > + (unless explicitly stated elsewhere) make any warranties or guarantees > + that it will achieve its stated purpose or function properly in any > + given situation. > + > + (e) I understand and agree that this project and the contribution are > + public and that a record of the contribution (including my Reviewed-by > + tag and any associated public communications) is maintained > + indefinitely and may be redistributed consistent with this project or > + the open source license(s) involved. --- (e) seems over-careful, especially since you're applying it only to the Review-by tag, while all the other tags would also have the same concern. -- scott preece - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, Jonathan Corbet [EMAIL PROTECTED] wrote: Some minor rewording suggestions: + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. --- Replace the last sentence with I am satisfied with the submitter's response to my comments. or The submitter has responded to my comments in a way that satisfied my concerns. --- + + (c) While there may (or may not) be things which could be improved with + this submission, I believe that it is, at this time, (1) a worthwhile + modification to the kernel, and (2) free of known issues which would + argue against its inclusion. --- I would suggest dropping the (or may not) as unnecessary, and changing the which would to that would. --- + + (d) While I have reviewed the patch and believe it to be sound, I can not --- From a legal standpoint, I do not might be preferable to I cannot, since it disclaims any intention to make such a statement, regardless of qualification. --- + (unless explicitly stated elsewhere) make any warranties or guarantees + that it will achieve its stated purpose or function properly in any + given situation. + + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. --- (e) seems over-careful, especially since you're applying it only to the Review-by tag, while all the other tags would also have the same concern. -- scott preece - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tue, Oct 09, 2007 at 10:49:20AM -0600, Jonathan Corbet wrote: > Neil Brown <[EMAIL PROTECTED]> wrote: > > > + (b) Any problems, concerns, or questions relating to the patch have been > > > + communicated back to the submitter. I am satisfied with how the > > > + submitter has responded to my comments. > > > > This seems more detailed that necessary. The process (communicated > > back / responded) is not really relevant. > > Instead, it seems to me that the process is crucially important. > Reviewed-by shouldn't be a rubber stamp that somebody applies to a > patch; I think it should really imply that issues of interest have been > communicated to the developers. If we are setting expectations for what > Reviewed-by means, I would prefer to leave an explicit mention of > communication in there. I couldn't agree more, Jon. If we are to have a meaningful reviewed-by tag, it has to be clearly documented as to what responsibilities it places on the reviewer. If someone doesn't want to perform a well conducted review, then they haven't earned the right to issue a Reviewed-by tag - they can use the Acked-by rubber stamp instead. FWIW, w.r.t. XFS patches, we already follow both the letter and intent of your proposed reviewed-by tag for all changes to XFS code and reviewers are currently listed as Signed-off-by in git-commits (our internal SCM records the reviewer(s) and the git export script converts that to s-o-b). It would be much more meaningful if they were exported as Reviewed-by under your definition IOWs, I fully support your definition of the Reviewed-by tag. Cheers, Dave. -- Dave Chinner Principal Engineer SGI Australian Software Group - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Hi Neil. > >From:The Author, Primary Author, or Authors of the patch. > Authors should also provide a Signed-off-by: tag. > > Purpose: to give credit to authors The SCM should include this info and we should not duplicate this in the changelog's. I know some tools require this format but that's something else. > > + > > +Signed-off-by: A person adding a Signed-off-by tag is attesting that the > > + patch is, to the best of his or her knowledge, legally able > > + to be merged into the mainline and distributed under the > > + terms of the GNU General Public License, version 2. See > > + the Developer's Certificate of Origin, found in > > + Documentation/SubmittingPatches, for the precise meaning of > > + Signed-off-by. > > Purpose: to allow subsequent review of the originality of > the contribution should copyright questions arise. We often use s-o-b to docuemnt the path a patch took from origin (the top-most s-o-b) to tree apply (lowest s-o-b). This is IIUC part of the intended behaviour of s-o-b but it is not clear from the above text. > > + > > +Acked-by: The person named (who should be an active developer in the > > + area addressed by the patch) is aware of the patch and has > > + no objection to its inclusion. An Acked-by tag does not > > + imply any involvement in the development of the patch or > > + that a detailed review was done. > > Purpose: to inform upstream aggregators that > consensus was achieved for the change. This is > particularly relevant for changes that affect multiple > Maintenance Domains. > consensus seems too strong a wording here. consensus imply more than one that agree on the patch where I often see people give their "Acked-by:" by simple changelog reading. So Acked-by: is not used like documented today. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
> > > + (b) Any problems, concerns, or questions relating to the patch have > > > been > > > + communicated back to the submitter. I am satisfied with how the > > > + submitter has responded to my comments. > > > > This seems more detailed that necessary. The process (communicated > > back / responded) is not really relevant. > > Instead, it seems to me that the process is crucially important. I agree with you Jon. In fact my first reaction to your initial post was that this section (b) was the most succinct distillation of the most important part of reviewing that I've seen. - R. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Neil Brown <[EMAIL PROTECTED]> wrote: > I find it is always good to know *why* we have the tags. That > information is a useful complement to what they mean, and can guide > people in adding them. Hmm...I was just going to go with the "because I told you so" approach that I use with my kids. It works so well with them after all. I agree with just about everything you've said, and am tweaking things accordingly. But... > > + (b) Any problems, concerns, or questions relating to the patch have been > > + communicated back to the submitter. I am satisfied with how the > > + submitter has responded to my comments. > > This seems more detailed that necessary. The process (communicated > back / responded) is not really relevant. Instead, it seems to me that the process is crucially important. Reviewed-by shouldn't be a rubber stamp that somebody applies to a patch; I think it should really imply that issues of interest have been communicated to the developers. If we are setting expectations for what Reviewed-by means, I would prefer to leave an explicit mention of communication in there. If I'm in the minority here, though, it can certainly come out. Thanks, jon - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 8 Oct 2007 19:30:54 -0400 "J. Bruce Fields" <[EMAIL PROTECTED]> wrote: > On Mon, Oct 08, 2007 at 04:43:10PM -0600, Jonathan Corbet wrote: > > + (e) I understand and agree that this project and the contribution are > > + public and that a record of the contribution (including my Reviewed-by > > + tag and any associated public communications) is maintained > > + indefinitely and may be redistributed consistent with this project or > > + the open source license(s) involved. > > Is this paragraph really necessary? (For example, is there some history > of problems that this is addressing?) It was added between 1.0 and 1.1 to ensure that there were no awkward interactions with various privacy and data protection laws by making sure the submitter understands it is a public record and is contributing on that basis. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
> Sam Ravnborg wrote: >> On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: >>> The SCM changelog should contain _what_ a patch does and if >>> necessary _why_ it does so. >> The _why_ part is more important than _what_. The diff should hopefully >> explain the _what_ part. > > "What": fix lockup in this and that circumstances > "Why": because lockups are annoying > "How": the diff > (That's what I meant with what and why.) PS, example with non-trivial why: What: add ABI which correlates bus cycle counter and local time Why: apps need it to sync streams from different buses How: the diff -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Sam Ravnborg wrote: > On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: >> The SCM changelog should contain _what_ a patch does and if >> necessary _why_ it does so. > The _why_ part is more important than _what_. The diff should hopefully > explain the _what_ part. "What": fix lockup in this and that circumstances "Why": because lockups are annoying "How": the diff (That's what I meant with what and why.) -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: > Steven Rostedt wrote: > > But for those that run test suites, they should be smart enough to put > > in more documentation into the change log to state how it was tested. > > I disagree. The SCM changelog should contain _what_ a patch does and if > necessary _why_ it does so. The _why_ part is more important than _what_. The diff should hopefully explain the _what_ part. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Steven Rostedt wrote: > But for those that run test suites, they should be smart enough to put > in more documentation into the change log to state how it was tested. I disagree. The SCM changelog should contain _what_ a patch does and if necessary _why_ it does so. The rest (e.g. the sign-off tag to state that the licensing is alright, and any other tags) should have its meaning sufficiently defined outside the changelog. Remember what the SCM changelog is for, i.e. what we do with it after commit. -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Steven Rostedt wrote: But for those that run test suites, they should be smart enough to put in more documentation into the change log to state how it was tested. I disagree. The SCM changelog should contain _what_ a patch does and if necessary _why_ it does so. The rest (e.g. the sign-off tag to state that the licensing is alright, and any other tags) should have its meaning sufficiently defined outside the changelog. Remember what the SCM changelog is for, i.e. what we do with it after commit. -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: Steven Rostedt wrote: But for those that run test suites, they should be smart enough to put in more documentation into the change log to state how it was tested. I disagree. The SCM changelog should contain _what_ a patch does and if necessary _why_ it does so. The _why_ part is more important than _what_. The diff should hopefully explain the _what_ part. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Sam Ravnborg wrote: On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: The SCM changelog should contain _what_ a patch does and if necessary _why_ it does so. The _why_ part is more important than _what_. The diff should hopefully explain the _what_ part. What: fix lockup in this and that circumstances Why: because lockups are annoying How: the diff (That's what I meant with what and why.) -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Sam Ravnborg wrote: On Tue, Oct 09, 2007 at 08:11:53AM +0200, Stefan Richter wrote: The SCM changelog should contain _what_ a patch does and if necessary _why_ it does so. The _why_ part is more important than _what_. The diff should hopefully explain the _what_ part. What: fix lockup in this and that circumstances Why: because lockups are annoying How: the diff (That's what I meant with what and why.) PS, example with non-trivial why: What: add ABI which correlates bus cycle counter and local time Why: apps need it to sync streams from different buses How: the diff -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 8 Oct 2007 19:30:54 -0400 J. Bruce Fields [EMAIL PROTECTED] wrote: On Mon, Oct 08, 2007 at 04:43:10PM -0600, Jonathan Corbet wrote: + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. Is this paragraph really necessary? (For example, is there some history of problems that this is addressing?) It was added between 1.0 and 1.1 to ensure that there were no awkward interactions with various privacy and data protection laws by making sure the submitter understands it is a public record and is contributing on that basis. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Neil Brown [EMAIL PROTECTED] wrote: I find it is always good to know *why* we have the tags. That information is a useful complement to what they mean, and can guide people in adding them. Hmm...I was just going to go with the because I told you so approach that I use with my kids. It works so well with them after all. pauses to go scream at his kids who have never understood why playing Dance Dance Revolution directly above the office is hard on productivity I agree with just about everything you've said, and am tweaking things accordingly. But... + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. Instead, it seems to me that the process is crucially important. Reviewed-by shouldn't be a rubber stamp that somebody applies to a patch; I think it should really imply that issues of interest have been communicated to the developers. If we are setting expectations for what Reviewed-by means, I would prefer to leave an explicit mention of communication in there. If I'm in the minority here, though, it can certainly come out. Thanks, jon - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
+ (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. Instead, it seems to me that the process is crucially important. I agree with you Jon. In fact my first reaction to your initial post was that this section (b) was the most succinct distillation of the most important part of reviewing that I've seen. - R. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Hi Neil. From:The Author, Primary Author, or Authors of the patch. Authors should also provide a Signed-off-by: tag. Purpose: to give credit to authors The SCM should include this info and we should not duplicate this in the changelog's. I know some tools require this format but that's something else. + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. Purpose: to allow subsequent review of the originality of the contribution should copyright questions arise. We often use s-o-b to docuemnt the path a patch took from origin (the top-most s-o-b) to tree apply (lowest s-o-b). This is IIUC part of the intended behaviour of s-o-b but it is not clear from the above text. + +Acked-by: The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. Purpose: to inform upstream aggregators that consensus was achieved for the change. This is particularly relevant for changes that affect multiple Maintenance Domains. consensus seems too strong a wording here. consensus imply more than one that agree on the patch where I often see people give their Acked-by: by simple changelog reading. So Acked-by: is not used like documented today. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Tue, Oct 09, 2007 at 10:49:20AM -0600, Jonathan Corbet wrote: Neil Brown [EMAIL PROTECTED] wrote: + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. Instead, it seems to me that the process is crucially important. Reviewed-by shouldn't be a rubber stamp that somebody applies to a patch; I think it should really imply that issues of interest have been communicated to the developers. If we are setting expectations for what Reviewed-by means, I would prefer to leave an explicit mention of communication in there. I couldn't agree more, Jon. If we are to have a meaningful reviewed-by tag, it has to be clearly documented as to what responsibilities it places on the reviewer. If someone doesn't want to perform a well conducted review, then they haven't earned the right to issue a Reviewed-by tag - they can use the Acked-by rubber stamp instead. FWIW, w.r.t. XFS patches, we already follow both the letter and intent of your proposed reviewed-by tag for all changes to XFS code and reviewers are currently listed as Signed-off-by in git-commits (our internal SCM records the reviewer(s) and the git export script converts that to s-o-b). It would be much more meaningful if they were exported as Reviewed-by under your definition IOWs, I fully support your definition of the Reviewed-by tag. Cheers, Dave. -- Dave Chinner Principal Engineer SGI Australian Software Group - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 8 Oct 2007 16:06:03 -0700 Randy Dunlap <[EMAIL PROTECTED]> wrote: > On Mon, 08 Oct 2007 16:43:10 -0600 Jonathan Corbet wrote: > > > Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > > > > Or maybe we need something much less formal that explain the purpose of > > > the > > > four tags we use: > > > > ...or maybe a combination? How does the following patch look as a way > > to describe how the tags are used and what Reviewed-by, in particular, > > means? > > > > Perhaps the DCO should move to this file as well? > > > > jon > > Just typos noted below... > > > --- > > > > Add a document on patch tags. > > > > Signed-off-by: Jonathan Corbet <[EMAIL PROTECTED]> > > > > diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX > > index 43e89b1..fa1518b 100644 > > --- a/Documentation/00-INDEX > > +++ b/Documentation/00-INDEX > > @@ -284,6 +284,8 @@ parport.txt > > - how to use the parallel-port driver. > > parport-lowlevel.txt > > - description and usage of the low level parallel port functions. > > +patch-tags > > + - description of the tags which can be added to patches > > pci-error-recovery.txt > > - info on PCI error recovery. > > pci.txt > > diff --git a/Documentation/patch-tags b/Documentation/patch-tags > > new file mode 100644 > > index 000..fb5f8e1 > > --- /dev/null > > +++ b/Documentation/patch-tags > > @@ -0,0 +1,66 @@ > > +Patches headed for the mainline may contain a variety of tags documenting > > +who played a hand in (or was at least aware of) its progress. All of these > > +tags have the form: > > + > > + Something-done-by: Full name <[EMAIL PROTECTED]> > > + > > +These tags are: > > + > > +Signed-off-by: A person adding a Signed-off-by tag is attesting that the > > + patch is, to the best of his or her knowledge, legally able > > + to be merged into the mainline and distributed under the > > + terms of the GNU General Public License, version 2. All changes are licensed under the terms of the file modified. (Some people seem not to understand that if the file is dual licensed, then the changes are dual licensed. If file is GPL v2 only, then the changes are GPL v2 only, ...) > > See > > + the Developer's Certificate of Origin, found in > > + Documentation/SubmittingPatches, for the precise meaning of > > + Signed-off-by. > > +Acked-by: The person named (who should be an active developer in the > > + area addressed by the patch) is aware of the patch and has > > + no objection to its inclusion. An Acked-by tag does not > > + imply any involvement in the development of the patch or > > + that a detailed review was done. > > + > > +Reviewed-by: The patch has been reviewed and found acceptible > > according > > acceptable > > > + to the Reviewer's Statement as found at the bottom of this > > + file. A Reviewed-by tag is a statement of opinion that the > > + patch is an appropriate modification of the kernel without > > + any remaining serious technical issues. Any interested > > + reviewer (who has done the work) can offer a Reviewed-by > > + tag for a patch. > > + > > +Cc:The person named was given the opportunity to comment on > > + the patch. This is the only tag which might be added > > + without an explicit action by the person it names. > > + > > +Tested-by: The patch has been successfully tested (in some > > + environment) by the person named. > > + > IMHO the other tags actually are a poor substitute for providing a more complete description of the reviewer's involvement. It would be better to have more complete responses like "the patch should be merged as is for 2.6.X but the following should be fixed, ..." etc. The certificate of origin has meaning for legal things that have a more concrete definition, but the existing process is about people making good (or bad) decisions based on feedback and other data. Trying to reduce the feedback down to 3 Acks, and 1 Review seems like noise. The problem is getting good reviews of new code in a timely manner, not the descriptions of the result. -- Stephen Hemminger <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 10:16:26PM +0200, Rafael J. Wysocki wrote: > > Tested-by: is sort of trivial for a fix patch, for example, if a bug reporter > confirms that the proposed patch actually fixes the issue. IMHO it wouldn't > be practical to complicate that. > I see two types of Tested-by. 1) As you stated, a fixed to a problem that the reporter has seen. So that someone could state a "fixes issue" in the change log and that would simple mean that the tester has seen a problem, and the attached patch fixes it. 2) Someone has a testsuite to the area that the change affects. So if someone has developed a networking test suite and a patch changes some networking logic, the Tested-by could be that the tester actually ran specific tests. This should require a more detail explaination of what was done. Or the very least, a pointer to a web page of the tests that were run. So for the user that sees an issue, then gets a patch, perhaps all they need to do is add a "fixed problem" or "works now" in the change log to denote that the patch has actually (or seems to) fix the problem that they previously seen. This shouldn't be too hard. But for those that run test suites, they should be smart enough to put in more documentation into the change log to state how it was tested. Perhaps we need to add yet another signed off. "Verified-by", which could be for the user that saw an issue and the patch now fixes it. That user could just add the "Verified-by" to the patch to acknowledge (and record) that the patch did fix the issue. The "Tested-by" can be used for patches that are run through a test suite. Just a thought. -- Steve - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday October 8, [EMAIL PROTECTED] wrote: I find it is always good to know *why* we have the tags. That information is a useful complement to what they mean, and can guide people in adding them. So below I present some "Purposes", YetAnotherTag, and a comment on the RSO. (And I'd like to add a vote for "Blame-Shared-By:" rather than "Reviewed-by:", however I don't I'll get much support...) > diff --git a/Documentation/patch-tags b/Documentation/patch-tags > new file mode 100644 > index 000..fb5f8e1 > --- /dev/null > +++ b/Documentation/patch-tags > @@ -0,0 +1,66 @@ > +Patches headed for the mainline may contain a variety of tags documenting > +who played a hand in (or was at least aware of) its progress. All of these > +tags have the form: > + > + Something-done-by: Full name <[EMAIL PROTECTED]> > + > +These tags are: From:The Author, Primary Author, or Authors of the patch. Authors should also provide a Signed-off-by: tag. Purpose: to give credit to authors > + > +Signed-off-by: A person adding a Signed-off-by tag is attesting that the > + patch is, to the best of his or her knowledge, legally able > + to be merged into the mainline and distributed under the > + terms of the GNU General Public License, version 2. See > + the Developer's Certificate of Origin, found in > + Documentation/SubmittingPatches, for the precise meaning of > + Signed-off-by. Purpose: to allow subsequent review of the originality of the contribution should copyright questions arise. > + > +Acked-by:The person named (who should be an active developer in the > + area addressed by the patch) is aware of the patch and has > + no objection to its inclusion. An Acked-by tag does not > + imply any involvement in the development of the patch or > + that a detailed review was done. Purpose: to inform upstream aggregators that consensus was achieved for the change. This is particularly relevant for changes that affect multiple Maintenance Domains. > + > +Reviewed-by: The patch has been reviewed and found acceptible according > + to the Reviewer's Statement as found at the bottom of this > + file. A Reviewed-by tag is a statement of opinion that the > + patch is an appropriate modification of the kernel without > + any remaining serious technical issues. Any interested > + reviewer (who has done the work) can offer a Reviewed-by > + tag for a patch. Purpose: to inform upstream aggregators that due diligence has been performed to ensure correctness of the change. Also to give credit to reviewers. > + > +Cc: The person named was given the opportunity to comment on > + the patch. This is the only tag which might be added > + without an explicit action by the person it names. Purpose: to ensure that interested parties are included in subsequent discussions of the change. > + > +Tested-by: The patch has been successfully tested (in some > + environment) by the person named. Purpose: to give credit to testers. > + > + > + > + > +Reviewer's statement of oversight, v0.02 > + > +By offering my Reviewed-by: tag, I state that: > + > + (a) I have carried out a technical review of this patch to evaluate its > + appropriateness and readiness for inclusion into the mainline kernel. > + > + (b) Any problems, concerns, or questions relating to the patch have been > + communicated back to the submitter. I am satisfied with how the > + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. I would go for something like: (b) I have no outstanding problems, concerns, or questions about this patch (except as noted in the above comments). and in fact, given (c2), (b) might not be needed at all. NeilBrown > + > + (c) While there may (or may not) be things which could be improved with > + this submission, I believe that it is, at this time, (1) a worthwhile > + modification to the kernel, and (2) free of known issues which would > + argue against its inclusion. > + > + (d) While I have reviewed the patch and believe it to be sound, I can not > + (unless explicitly stated elsewhere) make any warranties or guarantees > + that it will achieve its stated purpose or function properly in any > + given situation. > + > + (e) I understand and agree that this project and the contribution are > + public and that a record of the contribution (including my Reviewed-by > + tag and any associated public
Re: RFC: reviewer's statement of oversight
Jonathan Corbet wrote: > All of these > +tags have the form: > + > + Something-done-by: Full name <[EMAIL PROTECTED]> To be precise: Something-done-by: Full name <[EMAIL PROTECTED]> [optional random stuff] "Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off.", says SubmittingPatches. I actually do so on occasions. -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 04:43:10PM -0600, Jonathan Corbet wrote: > + (e) I understand and agree that this project and the contribution are > + public and that a record of the contribution (including my Reviewed-by > + tag and any associated public communications) is maintained > + indefinitely and may be redistributed consistent with this project or > + the open source license(s) involved. Is this paragraph really necessary? (For example, is there some history of problems that this is addressing?) --b. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 08 Oct 2007 16:43:10 -0600 Jonathan Corbet wrote: > Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > > Or maybe we need something much less formal that explain the purpose of the > > four tags we use: > > ...or maybe a combination? How does the following patch look as a way > to describe how the tags are used and what Reviewed-by, in particular, > means? > > Perhaps the DCO should move to this file as well? > > jon Just typos noted below... > --- > > Add a document on patch tags. > > Signed-off-by: Jonathan Corbet <[EMAIL PROTECTED]> > > diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX > index 43e89b1..fa1518b 100644 > --- a/Documentation/00-INDEX > +++ b/Documentation/00-INDEX > @@ -284,6 +284,8 @@ parport.txt > - how to use the parallel-port driver. > parport-lowlevel.txt > - description and usage of the low level parallel port functions. > +patch-tags > + - description of the tags which can be added to patches > pci-error-recovery.txt > - info on PCI error recovery. > pci.txt > diff --git a/Documentation/patch-tags b/Documentation/patch-tags > new file mode 100644 > index 000..fb5f8e1 > --- /dev/null > +++ b/Documentation/patch-tags > @@ -0,0 +1,66 @@ > +Patches headed for the mainline may contain a variety of tags documenting > +who played a hand in (or was at least aware of) its progress. All of these > +tags have the form: > + > + Something-done-by: Full name <[EMAIL PROTECTED]> > + > +These tags are: > + > +Signed-off-by: A person adding a Signed-off-by tag is attesting that the > + patch is, to the best of his or her knowledge, legally able > + to be merged into the mainline and distributed under the > + terms of the GNU General Public License, version 2. See > + the Developer's Certificate of Origin, found in > + Documentation/SubmittingPatches, for the precise meaning of > + Signed-off-by. > + > +Acked-by:The person named (who should be an active developer in the > + area addressed by the patch) is aware of the patch and has > + no objection to its inclusion. An Acked-by tag does not > + imply any involvement in the development of the patch or > + that a detailed review was done. > + > +Reviewed-by: The patch has been reviewed and found acceptible according acceptable > + to the Reviewer's Statement as found at the bottom of this > + file. A Reviewed-by tag is a statement of opinion that the > + patch is an appropriate modification of the kernel without > + any remaining serious technical issues. Any interested > + reviewer (who has done the work) can offer a Reviewed-by > + tag for a patch. > + > +Cc: The person named was given the opportunity to comment on > + the patch. This is the only tag which might be added > + without an explicit action by the person it names. > + > +Tested-by: The patch has been successfully tested (in some > + environment) by the person named. > + > + > + > + > +Reviewer's statement of oversight, v0.02 > + > +By offering my Reviewed-by: tag, I state that: > + > + (a) I have carried out a technical review of this patch to evaluate its > + appropriateness and readiness for inclusion into the mainline kernel. > + > + (b) Any problems, concerns, or questions relating to the patch have been > + communicated back to the submitter. I am satisfied with how the > + submitter has responded to my comments. > + > + (c) While there may (or may not) be things which could be improved with > + this submission, I believe that it is, at this time, (1) a worthwhile > + modification to the kernel, and (2) free of known issues which would > + argue against its inclusion. > + > + (d) While I have reviewed the patch and believe it to be sound, I can not cannot > + (unless explicitly stated elsewhere) make any warranties or guarantees > + that it will achieve its stated purpose or function properly in any > + given situation. > + > + (e) I understand and agree that this project and the contribution are > + public and that a record of the contribution (including my Reviewed-by > + tag and any associated public communications) is maintained > + indefinitely and may be redistributed consistent with this project or > + the open source license(s) involved. > - --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
* Mon, 8 Oct 2007 17:38:52 -0400 > > On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: >> Uhm, no. There is no reason an "unimportant" person couldn't review a >> patch, and therefore perform a potentially highly valuable service to >> the maintainer. >> >> None of these are indicative of the authority of the person acking, >> reviewing, testing, or nacking. That's only as good as the trust in the >> person signing. > > I would tend to agree. Right now I think the problem is that we are > getting too little reviews, not enough. And someone who reviews > patches, even if unknown, could be building up expertise that > eventually would make them a valued developer, even while they are > doing us a service. Experience of convincing experienced patch author, that some things in the patch are wrong :) [] > We could ask reviewers to include a URL to an LKML archive of their > review, to make it easier to find a review of a patch so later on > people can judge how effective they their review was. I vote for more little summaries in the `Subject'(again). Long, boring threads with whole threading part of screen being empty due to same subjects isn't fun, when some of thousands of messages can have interesting stuff inside. And it's easy not only for mailing list readers now, and for archive readers also; readers of the www search results (who ever that may be): google.com/search?q=reviewed+crashkernel First hit on the review of the patch, i happened to make. And i just thought "hell, just string parsing, what can be more simply?", yet there was productive discussion and bug fixing. After i saw convincing statements about testing, i've placed review mark. Though i'm really "unimportant" random hacker. -- -o--=O`C #oo'L O <___=E M - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Sam Ravnborg <[EMAIL PROTECTED]> wrote: > Or maybe we need something much less formal that explain the purpose of the > four tags we use: ...or maybe a combination? How does the following patch look as a way to describe how the tags are used and what Reviewed-by, in particular, means? Perhaps the DCO should move to this file as well? jon --- Add a document on patch tags. Signed-off-by: Jonathan Corbet <[EMAIL PROTECTED]> diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX index 43e89b1..fa1518b 100644 --- a/Documentation/00-INDEX +++ b/Documentation/00-INDEX @@ -284,6 +284,8 @@ parport.txt - how to use the parallel-port driver. parport-lowlevel.txt - description and usage of the low level parallel port functions. +patch-tags + - description of the tags which can be added to patches pci-error-recovery.txt - info on PCI error recovery. pci.txt diff --git a/Documentation/patch-tags b/Documentation/patch-tags new file mode 100644 index 000..fb5f8e1 --- /dev/null +++ b/Documentation/patch-tags @@ -0,0 +1,66 @@ +Patches headed for the mainline may contain a variety of tags documenting +who played a hand in (or was at least aware of) its progress. All of these +tags have the form: + + Something-done-by: Full name <[EMAIL PROTECTED]> + +These tags are: + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. + +Acked-by: The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. + +Reviewed-by: The patch has been reviewed and found acceptible according + to the Reviewer's Statement as found at the bottom of this + file. A Reviewed-by tag is a statement of opinion that the + patch is an appropriate modification of the kernel without + any remaining serious technical issues. Any interested + reviewer (who has done the work) can offer a Reviewed-by + tag for a patch. + +Cc:The person named was given the opportunity to comment on + the patch. This is the only tag which might be added + without an explicit action by the person it names. + +Tested-by: The patch has been successfully tested (in some + environment) by the person named. + + + + +Reviewer's statement of oversight, v0.02 + +By offering my Reviewed-by: tag, I state that: + + (a) I have carried out a technical review of this patch to evaluate its + appropriateness and readiness for inclusion into the mainline kernel. + + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. + + (c) While there may (or may not) be things which could be improved with + this submission, I believe that it is, at this time, (1) a worthwhile + modification to the kernel, and (2) free of known issues which would + argue against its inclusion. + + (d) While I have reviewed the patch and believe it to be sound, I can not + (unless explicitly stated elsewhere) make any warranties or guarantees + that it will achieve its stated purpose or function properly in any + given situation. + + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday, 8 October 2007 23:38, Theodore Tso wrote: > On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: > > Uhm, no. There is no reason an "unimportant" person couldn't review a > > patch, and therefore perform a potentially highly valuable service to > > the maintainer. > > > > None of these are indicative of the authority of the person acking, > > reviewing, testing, or nacking. That's only as good as the trust in the > > person signing. > > I would tend to agree. Right now I think the problem is that we are > getting too little reviews, not enough. And someone who reviews > patches, even if unknown, could be building up expertise that > eventually would make them a valued developer, even while they are > doing us a service. > > The concern that I suspect some people have is what if this gets > abused by people who don't really bother to do a full review of a > patch before they ack it. We could ask reviewers to include a URL to > an LKML archive of their review, to make it easier to find a review of > a patch so later on people can judge how effective they their review > was. Unfortunately, this would be an added burden for the regular > reviewers, so I doubt this would be well accepted as a practice. My > suggestion is to not worry about this for now, and see how well it > works out in practice. If we start getting half a dozen or more > Reviewed-by: where the patch is pretty clearly not getting adequately > reviewed, or where someone is obviously abusing the system, and social > pressures aren't working, we can try to figure out then how we want to > address that problem then. Let's not make the process too complicated > unless we know it's necessary. Premature complexity is almost as bad > as premature optimization I agree. Greetings, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: > Uhm, no. There is no reason an "unimportant" person couldn't review a > patch, and therefore perform a potentially highly valuable service to > the maintainer. > > None of these are indicative of the authority of the person acking, > reviewing, testing, or nacking. That's only as good as the trust in the > person signing. I would tend to agree. Right now I think the problem is that we are getting too little reviews, not enough. And someone who reviews patches, even if unknown, could be building up expertise that eventually would make them a valued developer, even while they are doing us a service. The concern that I suspect some people have is what if this gets abused by people who don't really bother to do a full review of a patch before they ack it. We could ask reviewers to include a URL to an LKML archive of their review, to make it easier to find a review of a patch so later on people can judge how effective they their review was. Unfortunately, this would be an added burden for the regular reviewers, so I doubt this would be well accepted as a practice. My suggestion is to not worry about this for now, and see how well it works out in practice. If we start getting half a dozen or more Reviewed-by: where the patch is pretty clearly not getting adequately reviewed, or where someone is obviously abusing the system, and social pressures aren't working, we can try to figure out then how we want to address that problem then. Let's not make the process too complicated unless we know it's necessary. Premature complexity is almost as bad as premature optimization - Ted - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth Uhm, no. There is no reason an "unimportant" person couldn't review a patch, and therefore perform a potentially highly valuable service to the maintainer. None of these are indicative of the authority of the person acking, reviewing, testing, or nacking. That's only as good as the trust in the person signing. -hpa - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday, 8 October 2007 21:26, Scott Preece wrote: > On 10/8/07, J. Bruce Fields <[EMAIL PROTECTED]> wrote: > > On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: > ... > > > So, putting a Tested-by into the changelog is only useful if the > > > necessary testing is rather simple (i.e. "fixed the bug which I was > > > always able to reproduce before") or if the tester is known to have > > > performed rigorous and sufficiently broad tests. > > > > Well, you can still include those test-method details in the body of the > > message in addition to adding the "Tested-by:". > > > > Does "Tested-by" just mean they ran some relevant test on the final > > version of the patch? The really hard part is often the initial work > > required to find a good reproduceable test case, capture the right error > > report, or bisect to the right commit. I think that also counts as > > "testing". And it'd be nice to have a tag for those sorts of > > contributions, partly just as another way to ackowledge them. > --- > > Tested-by should, at the very least, have a description of the test > environment in the body (suggesting that the change compiled and ran > in that environment). Preferably it should also have a description of > the test or test suite run and whether that test failed on an > unpatched system. Tested-by: is sort of trivial for a fix patch, for example, if a bug reporter confirms that the proposed patch actually fixes the issue. IMHO it wouldn't be practical to complicate that. Greetings, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, Jan Engelhardt <[EMAIL PROTECTED]> wrote: > > On Oct 8 2007 19:37, Sam Ravnborg wrote: > >snip... > >Acked-by: > >Tested-by: > > * Used by random people to express their (dis)like/experience with the > patch. > > >Reviewed-by: > > * I am maintaner or an 'important' person and have had a > look at it in depth --- A couple of months ago there was a thread about Acked-by. At that time the consensus seemed to be that it was inappropriate for random people to add Acked-by - that it should only be added by people whose opinion was expected to a gate for merging. Did the Summit reach a decision that Acked-by was for anybody and Reviewed-by was for authorities? I like Randy's point - the assigning of weight to the Reviewer's opinion is necessarily in the hands of those at the top of the merge process and there's no need to ask reviewers to decide whether their opinion matters. In that view, "Acked-by" means "I have no objection to this patch, but don't claim deep review" and "Reviewed-by" means "I have no objection to this patch after a thorough review", and either can be attached by anybody who wants to express an opinion. scott -- scott preece - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, J. Bruce Fields <[EMAIL PROTECTED]> wrote: > On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: ... > > So, putting a Tested-by into the changelog is only useful if the > > necessary testing is rather simple (i.e. "fixed the bug which I was > > always able to reproduce before") or if the tester is known to have > > performed rigorous and sufficiently broad tests. > > Well, you can still include those test-method details in the body of the > message in addition to adding the "Tested-by:". > > Does "Tested-by" just mean they ran some relevant test on the final > version of the patch? The really hard part is often the initial work > required to find a good reproduceable test case, capture the right error > report, or bisect to the right commit. I think that also counts as > "testing". And it'd be nice to have a tag for those sorts of > contributions, partly just as another way to ackowledge them. --- Tested-by should, at the very least, have a description of the test environment in the body (suggesting that the change compiled and ran in that environment). Preferably it should also have a description of the test or test suite run and whether that test failed on an unpatched system. scott -- scott preece - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Al Viro <[EMAIL PROTECTED]> wrote: > > A patch which is not "worthwhile" is also not "appropriate". Mere > > correctness in a mathematical sense is not enough as technical review > > criterion. > > Yes, but there's also such thing as "worthwhile removal". Good point. So the text should probably say "worthwhile change" rather than "worthwhile addition." I do believe that thinking about whether the change as a whole is a desirable thing is an important part of the review process. jon - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
J. Bruce Fields wrote: > And it'd be nice to have a tag for those sorts of > contributions, partly just as another way to ackowledge them. Yes, although the primary purpose of the various tags should be to document the quality assurance process, or how to call it. However, what belongs into the SCM changelog, and what can the list archives provide? -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 08:53:05PM +0200, Stefan Richter wrote: > Mark Gross wrote: > > On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: > >> (a) I have carried out a technical review of this patch to evaluate its > >> appropriateness and readiness for inclusion into the mainline kernel. > [...] > >> (c) While there may (or may not) be things which could be improved with > >> this submission, I believe that it is, at this time, (1) a > >> worthwhile addition to the kernel, and (2) free of serious known > >> issues which would argue against its inclusion. > > > > C-1 "worthwhile addition..." Probably shouldn't be part of this. That's > > what additional Signed off by ACK's provide. I think reviewed by should > > limit its scope to code correctness leaving the subjective "worthwhile" > > statements are better expressed with other tags. > > A patch which is not "worthwhile" is also not "appropriate". Mere > correctness in a mathematical sense is not enough as technical review > criterion. Yes, but there's also such thing as "worthwhile removal". - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: > Randy Dunlap wrote: > > On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: > >> Tested-by is more valuable than acked-by, because its empirical. > >> Acked-by generally means "I don't generally object to the idea of the > >> patch, but may not have read beyond the changelog". Tested-by implies > >> "I did something that exercised the patch, and it didn't explode" - > >> that's on par with an actual review (ideally all patches would be both > >> tested and reviewed). > > > > but Tested-by: doesn't have to involve any "actually looking at/reading > > the patch." Right? > > > > IOW, the patch could be ugly as sin but it works... > > Tested-by translated into German and back into English: "Works for me, > test methods not specified." > > So, putting a Tested-by into the changelog is only useful if the > necessary testing is rather simple (i.e. "fixed the bug which I was > always able to reproduce before") or if the tester is known to have > performed rigorous and sufficiently broad tests. Well, you can still include those test-method details in the body of the message in addition to adding the "Tested-by:". Does "Tested-by" just mean they ran some relevant test on the final version of the patch? The really hard part is often the initial work required to find a good reproduceable test case, capture the right error report, or bisect to the right commit. I think that also counts as "testing". And it'd be nice to have a tag for those sorts of contributions, partly just as another way to ackowledge them. --b. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Mark Gross wrote: > On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: >> (a) I have carried out a technical review of this patch to evaluate its >> appropriateness and readiness for inclusion into the mainline kernel. [...] >> (c) While there may (or may not) be things which could be improved with >> this submission, I believe that it is, at this time, (1) a >> worthwhile addition to the kernel, and (2) free of serious known >> issues which would argue against its inclusion. > > C-1 "worthwhile addition..." Probably shouldn't be part of this. That's > what additional Signed off by ACK's provide. I think reviewed by should > limit its scope to code correctness leaving the subjective "worthwhile" > statements are better expressed with other tags. A patch which is not "worthwhile" is also not "appropriate". Mere correctness in a mathematical sense is not enough as technical review criterion. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: > Last month, at the kernel summit, there was discussion of putting a > Reviewed-by: tag onto patches to document the oversight they had > received on their way into the mainline. That tag has made an > occasional appearance since then, but there has not yet been a > discussion of what it really means. So it has not yet brought a whole > lot of value to the process. > > As I was trying to sleep last night, it occurred to me that what we > might need is an equivalent of the DCO for the Reviewed-by tag. To that > end, I dedicated a few minutes of my life to the following bit of text. > It's really just meant to be a starting point for the discussion. Is > the following something close to what we understand Reviewed-by to mean? > > jon > > > Reviewer's statement of oversight v0.01 > > By offering my Reviewed-by: tag, I state that: > > (a) I have carried out a technical review of this patch to evaluate its > appropriateness and readiness for inclusion into the mainline kernel. > > (b) Any problems, concerns, or questions relating to the patch have been > communicated back to the submitter. I am satisfied with how the > submitter has responded to my comments. > > (c) While there may (or may not) be things which could be improved with > this submission, I believe that it is, at this time, (1) a > worthwhile addition to the kernel, and (2) free of serious known > issues which would argue against its inclusion. C-1 "worthwhile addition..." Probably shouldn't be part of this. That's what additional Signed off by ACK's provide. I think reviewed by should limit its scope to code correctness leaving the subjective "worthwhile" statements are better expressed with other tags. > > (d) While I have reviewed the patch and believe it to be sound, I can not > (unless explicitly stated elsewhere) make any warranties or guarantees > that it will achieve its stated purpose or function properly in any > given situation. > > (e) I understand and agree that this project and the contribution are > public and that a record of the contribution (including my Reviewed-by > tag and any associated public communications) is maintained > indefinitely and may be redistributed consistent with this project or > the open source license(s) involved. > - I think this is a good thing to have, although recruiting reviews remains an open issue. I think it would be easier to recruit patch testers than reviewers should a Tested-by: tag be considered as well? --mgross - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
> >Reviewed-by: > > * I am maintaner or an 'important' person and have had a > look at it in depth I think anyone should be able to supply a Reviewed-by: tag no matter who they are. Of course the weight that such a tag carries in the final decision of whether or not to merge may depend on who supplied it, but I don't think we should try to formalize that aspect. In other words, I would define Reviewed-by: as just "I have looked at this patch in depth." Jon's text seems like a really good summation of that idea in more explicit language. - R. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Randy Dunlap wrote: > On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: >> Tested-by is more valuable than acked-by, because its empirical. >> Acked-by generally means "I don't generally object to the idea of the >> patch, but may not have read beyond the changelog". Tested-by implies >> "I did something that exercised the patch, and it didn't explode" - >> that's on par with an actual review (ideally all patches would be both >> tested and reviewed). > > but Tested-by: doesn't have to involve any "actually looking at/reading > the patch." Right? > > IOW, the patch could be ugly as sin but it works... Tested-by translated into German and back into English: "Works for me, test methods not specified." So, putting a Tested-by into the changelog is only useful if the necessary testing is rather simple (i.e. "fixed the bug which I was always able to reproduce before") or if the tester is known to have performed rigorous and sufficiently broad tests. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: > On Oct 8 2007 19:37, Sam Ravnborg wrote: >> snip... >> >> Or maybe we need something much less formal that explain the purpose of the >> four tags we use: > > At least formal try: > >> Signed-of-by: > > * Used by original submitter(s). > * Also used by maintainers to track the patch's path > (ATM, does not imply "I have look at it in depth") Signed-off-by (as far as the Developer's Certificate of Origin defines it) says: I made sure that this patch complies with all involved licenses. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Randy Dunlap wrote: > but Tested-by: doesn't have to involve any "actually looking at/reading > the patch." Right? > > IOW, the patch could be ugly as sin but it works... > Sure, absolutely. I never said its a substitute for review. An ugly working patch is useful, because its the raw material for a nice working patch. A nice non-working patch can be framed and admired from a distance, but it isn't terribly useful. J - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: > Jan Engelhardt wrote: > >> Acked-by: > >> Tested-by: > >> > > > > * Used by random people to express their (dis)like/experience with the > > patch. > > > > Tested-by is more valuable than acked-by, because its empirical. > Acked-by generally means "I don't generally object to the idea of the > patch, but may not have read beyond the changelog". Tested-by implies > "I did something that exercised the patch, and it didn't explode" - > that's on par with an actual review (ideally all patches would be both > tested and reviewed). but Tested-by: doesn't have to involve any "actually looking at/reading the patch." Right? IOW, the patch could be ugly as sin but it works... > >> Reviewed-by: > >> > > > > * I am maintaner or an 'important' person and have had a > > look at it in depth > > > > Hm. We have a tension here: > > * there aren't enough reviewers > * some reviews are more useful than others > > While a review by a trustworthy person is invaluable, we don't want to > discourage people from reviewing. A new reviewer's review may not be > terribly useful, but a meta-review may help improve it. Or it could be > a great review. > > I guess I'm proposing that we also need to expand the reviewer base, and > to do so we need some kind of reviewer-mentoring or metareview process. > Of course that could just be an extra burden on the existing (small) > trusted reviewer base, but the hope is that over time the reviewer pool > size grows enough to make the effort worthwhile... > > > >> Cc: > >> > > > > * Used by original submitter to denote additional maintainers it goes to > > * Parties who should be Cced when an a posteriori question comes up > > > > Well, any interested parties, really. I use it for original bug > reporters, people who followed up on the report, people who have patches > in a nearby area, people who are known to be interested in the affected > subsystem, people who have reviewed previous versions of the patch, etc... --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: >> Acked-by: >> Tested-by: >> > > * Used by random people to express their (dis)like/experience with the > patch. > Tested-by is more valuable than acked-by, because its empirical. Acked-by generally means "I don't generally object to the idea of the patch, but may not have read beyond the changelog". Tested-by implies "I did something that exercised the patch, and it didn't explode" - that's on par with an actual review (ideally all patches would be both tested and reviewed). >> Reviewed-by: >> > > * I am maintaner or an 'important' person and have had a > look at it in depth > Hm. We have a tension here: * there aren't enough reviewers * some reviews are more useful than others While a review by a trustworthy person is invaluable, we don't want to discourage people from reviewing. A new reviewer's review may not be terribly useful, but a meta-review may help improve it. Or it could be a great review. I guess I'm proposing that we also need to expand the reviewer base, and to do so we need some kind of reviewer-mentoring or metareview process. Of course that could just be an extra burden on the existing (small) trusted reviewer base, but the hope is that over time the reviewer pool size grows enough to make the effort worthwhile... >> Cc: >> > > * Used by original submitter to denote additional maintainers it goes to > * Parties who should be Cced when an a posteriori question comes up > Well, any interested parties, really. I use it for original bug reporters, people who followed up on the report, people who have patches in a nearby area, people who are known to be interested in the affected subsystem, people who have reviewed previous versions of the patch, etc... J - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Oct 8 2007 19:37, Sam Ravnborg wrote: >snip... > >Or maybe we need something much less formal that explain the purpose of the >four tags we use: At least formal try: >Signed-of-by: * Used by original submitter(s). * Also used by maintainers to track the patch's path (ATM, does not imply "I have look at it in depth") >Acked-by: >Tested-by: * Used by random people to express their (dis)like/experience with the patch. >Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth >Cc: * Used by original submitter to denote additional maintainers it goes to * Parties who should be Cced when an a posteriori question comes up My 2¥. Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: > Last month, at the kernel summit, there was discussion of putting a > Reviewed-by: tag onto patches to document the oversight they had > received on their way into the mainline. That tag has made an > occasional appearance since then, but there has not yet been a > discussion of what it really means. So it has not yet brought a whole > lot of value to the process. > > As I was trying to sleep last night, it occurred to me that what we > might need is an equivalent of the DCO for the Reviewed-by tag. To that > end, I dedicated a few minutes of my life to the following bit of text. > It's really just meant to be a starting point for the discussion. Is > the following something close to what we understand Reviewed-by to mean? > > jon > > > Reviewer's statement of oversight v0.01 > > By offering my Reviewed-by: tag, I state that: snip... Or maybe we need something much less formal that explain the purpose of the four tags we use: Signed-of-by: Acked-by: Reviewed-by: Cc: Tested-by: OK - make it five then. I continously to see people mixing up especially Acked-by: and Signed-of-by: both here at lkml but especially at the arm-kernel list (the only other Linux dev list I follow atm). I do beleive we see similar pattern in the other linux-dev lists where people are confused by these tags and need a short two line summery for each of them. Sam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Hi Jonathan, On 10/8/07, Jonathan Corbet <[EMAIL PROTECTED]> wrote: > As I was trying to sleep last night, it occurred to me that what we > might need is an equivalent of the DCO for the Reviewed-by tag. To that > end, I dedicated a few minutes of my life to the following bit of text. > It's really just meant to be a starting point for the discussion. Is > the following something close to what we understand Reviewed-by to mean? [snip] Looks good but how is this different from the Acked-by tag we are already using? Pekka - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Hi Jonathan, On 10/8/07, Jonathan Corbet [EMAIL PROTECTED] wrote: As I was trying to sleep last night, it occurred to me that what we might need is an equivalent of the DCO for the Reviewed-by tag. To that end, I dedicated a few minutes of my life to the following bit of text. It's really just meant to be a starting point for the discussion. Is the following something close to what we understand Reviewed-by to mean? [snip] Looks good but how is this different from the Acked-by tag we are already using? Pekka - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: Last month, at the kernel summit, there was discussion of putting a Reviewed-by: tag onto patches to document the oversight they had received on their way into the mainline. That tag has made an occasional appearance since then, but there has not yet been a discussion of what it really means. So it has not yet brought a whole lot of value to the process. As I was trying to sleep last night, it occurred to me that what we might need is an equivalent of the DCO for the Reviewed-by tag. To that end, I dedicated a few minutes of my life to the following bit of text. It's really just meant to be a starting point for the discussion. Is the following something close to what we understand Reviewed-by to mean? jon Reviewer's statement of oversight v0.01 By offering my Reviewed-by: tag, I state that: snip... Or maybe we need something much less formal that explain the purpose of the four tags we use: Signed-of-by: Acked-by: Reviewed-by: Cc: Tested-by: OK - make it five then. I continously to see people mixing up especially Acked-by: and Signed-of-by: both here at lkml but especially at the arm-kernel list (the only other Linux dev list I follow atm). I do beleive we see similar pattern in the other linux-dev lists where people are confused by these tags and need a short two line summery for each of them. Sam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Oct 8 2007 19:37, Sam Ravnborg wrote: snip... Or maybe we need something much less formal that explain the purpose of the four tags we use: At least formal try: Signed-of-by: * Used by original submitter(s). * Also used by maintainers to track the patch's path (ATM, does not imply I have look at it in depth) Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth Cc: * Used by original submitter to denote additional maintainers it goes to * Parties who should be Cced when an a posteriori question comes up My 2¥. Jan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Tested-by is more valuable than acked-by, because its empirical. Acked-by generally means I don't generally object to the idea of the patch, but may not have read beyond the changelog. Tested-by implies I did something that exercised the patch, and it didn't explode - that's on par with an actual review (ideally all patches would be both tested and reviewed). Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth Hm. We have a tension here: * there aren't enough reviewers * some reviews are more useful than others While a review by a trustworthy person is invaluable, we don't want to discourage people from reviewing. A new reviewer's review may not be terribly useful, but a meta-review may help improve it. Or it could be a great review. I guess I'm proposing that we also need to expand the reviewer base, and to do so we need some kind of reviewer-mentoring or metareview process. Of course that could just be an extra burden on the existing (small) trusted reviewer base, but the hope is that over time the reviewer pool size grows enough to make the effort worthwhile... Cc: * Used by original submitter to denote additional maintainers it goes to * Parties who should be Cced when an a posteriori question comes up Well, any interested parties, really. I use it for original bug reporters, people who followed up on the report, people who have patches in a nearby area, people who are known to be interested in the affected subsystem, people who have reviewed previous versions of the patch, etc... J - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: Jan Engelhardt wrote: Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Tested-by is more valuable than acked-by, because its empirical. Acked-by generally means I don't generally object to the idea of the patch, but may not have read beyond the changelog. Tested-by implies I did something that exercised the patch, and it didn't explode - that's on par with an actual review (ideally all patches would be both tested and reviewed). but Tested-by: doesn't have to involve any actually looking at/reading the patch. Right? IOW, the patch could be ugly as sin but it works... Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth Hm. We have a tension here: * there aren't enough reviewers * some reviews are more useful than others While a review by a trustworthy person is invaluable, we don't want to discourage people from reviewing. A new reviewer's review may not be terribly useful, but a meta-review may help improve it. Or it could be a great review. I guess I'm proposing that we also need to expand the reviewer base, and to do so we need some kind of reviewer-mentoring or metareview process. Of course that could just be an extra burden on the existing (small) trusted reviewer base, but the hope is that over time the reviewer pool size grows enough to make the effort worthwhile... Cc: * Used by original submitter to denote additional maintainers it goes to * Parties who should be Cced when an a posteriori question comes up Well, any interested parties, really. I use it for original bug reporters, people who followed up on the report, people who have patches in a nearby area, people who are known to be interested in the affected subsystem, people who have reviewed previous versions of the patch, etc... --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Randy Dunlap wrote: but Tested-by: doesn't have to involve any actually looking at/reading the patch. Right? IOW, the patch could be ugly as sin but it works... Sure, absolutely. I never said its a substitute for review. An ugly working patch is useful, because its the raw material for a nice working patch. A nice non-working patch can be framed and admired from a distance, but it isn't terribly useful. J - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: On Oct 8 2007 19:37, Sam Ravnborg wrote: snip... Or maybe we need something much less formal that explain the purpose of the four tags we use: At least formal try: Signed-of-by: * Used by original submitter(s). * Also used by maintainers to track the patch's path (ATM, does not imply I have look at it in depth) Signed-off-by (as far as the Developer's Certificate of Origin defines it) says: I made sure that this patch complies with all involved licenses. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Randy Dunlap wrote: On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: Tested-by is more valuable than acked-by, because its empirical. Acked-by generally means I don't generally object to the idea of the patch, but may not have read beyond the changelog. Tested-by implies I did something that exercised the patch, and it didn't explode - that's on par with an actual review (ideally all patches would be both tested and reviewed). but Tested-by: doesn't have to involve any actually looking at/reading the patch. Right? IOW, the patch could be ugly as sin but it works... Tested-by translated into German and back into English: Works for me, test methods not specified. So, putting a Tested-by into the changelog is only useful if the necessary testing is rather simple (i.e. fixed the bug which I was always able to reproduce before) or if the tester is known to have performed rigorous and sufficiently broad tests. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth I think anyone should be able to supply a Reviewed-by: tag no matter who they are. Of course the weight that such a tag carries in the final decision of whether or not to merge may depend on who supplied it, but I don't think we should try to formalize that aspect. In other words, I would define Reviewed-by: as just I have looked at this patch in depth. Jon's text seems like a really good summation of that idea in more explicit language. - R. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: Last month, at the kernel summit, there was discussion of putting a Reviewed-by: tag onto patches to document the oversight they had received on their way into the mainline. That tag has made an occasional appearance since then, but there has not yet been a discussion of what it really means. So it has not yet brought a whole lot of value to the process. As I was trying to sleep last night, it occurred to me that what we might need is an equivalent of the DCO for the Reviewed-by tag. To that end, I dedicated a few minutes of my life to the following bit of text. It's really just meant to be a starting point for the discussion. Is the following something close to what we understand Reviewed-by to mean? jon Reviewer's statement of oversight v0.01 By offering my Reviewed-by: tag, I state that: (a) I have carried out a technical review of this patch to evaluate its appropriateness and readiness for inclusion into the mainline kernel. (b) Any problems, concerns, or questions relating to the patch have been communicated back to the submitter. I am satisfied with how the submitter has responded to my comments. (c) While there may (or may not) be things which could be improved with this submission, I believe that it is, at this time, (1) a worthwhile addition to the kernel, and (2) free of serious known issues which would argue against its inclusion. C-1 worthwhile addition... Probably shouldn't be part of this. That's what additional Signed off by ACK's provide. I think reviewed by should limit its scope to code correctness leaving the subjective worthwhile statements are better expressed with other tags. (d) While I have reviewed the patch and believe it to be sound, I can not (unless explicitly stated elsewhere) make any warranties or guarantees that it will achieve its stated purpose or function properly in any given situation. (e) I understand and agree that this project and the contribution are public and that a record of the contribution (including my Reviewed-by tag and any associated public communications) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. - I think this is a good thing to have, although recruiting reviews remains an open issue. I think it would be easier to recruit patch testers than reviewers should a Tested-by: tag be considered as well? --mgross - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Mark Gross wrote: On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: (a) I have carried out a technical review of this patch to evaluate its appropriateness and readiness for inclusion into the mainline kernel. [...] (c) While there may (or may not) be things which could be improved with this submission, I believe that it is, at this time, (1) a worthwhile addition to the kernel, and (2) free of serious known issues which would argue against its inclusion. C-1 worthwhile addition... Probably shouldn't be part of this. That's what additional Signed off by ACK's provide. I think reviewed by should limit its scope to code correctness leaving the subjective worthwhile statements are better expressed with other tags. A patch which is not worthwhile is also not appropriate. Mere correctness in a mathematical sense is not enough as technical review criterion. -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: Randy Dunlap wrote: On Mon, 08 Oct 2007 11:01:49 -0700 Jeremy Fitzhardinge wrote: Tested-by is more valuable than acked-by, because its empirical. Acked-by generally means I don't generally object to the idea of the patch, but may not have read beyond the changelog. Tested-by implies I did something that exercised the patch, and it didn't explode - that's on par with an actual review (ideally all patches would be both tested and reviewed). but Tested-by: doesn't have to involve any actually looking at/reading the patch. Right? IOW, the patch could be ugly as sin but it works... Tested-by translated into German and back into English: Works for me, test methods not specified. So, putting a Tested-by into the changelog is only useful if the necessary testing is rather simple (i.e. fixed the bug which I was always able to reproduce before) or if the tester is known to have performed rigorous and sufficiently broad tests. Well, you can still include those test-method details in the body of the message in addition to adding the Tested-by:. Does Tested-by just mean they ran some relevant test on the final version of the patch? The really hard part is often the initial work required to find a good reproduceable test case, capture the right error report, or bisect to the right commit. I think that also counts as testing. And it'd be nice to have a tag for those sorts of contributions, partly just as another way to ackowledge them. --b. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 08:53:05PM +0200, Stefan Richter wrote: Mark Gross wrote: On Mon, Oct 08, 2007 at 11:24:45AM -0600, Jonathan Corbet wrote: (a) I have carried out a technical review of this patch to evaluate its appropriateness and readiness for inclusion into the mainline kernel. [...] (c) While there may (or may not) be things which could be improved with this submission, I believe that it is, at this time, (1) a worthwhile addition to the kernel, and (2) free of serious known issues which would argue against its inclusion. C-1 worthwhile addition... Probably shouldn't be part of this. That's what additional Signed off by ACK's provide. I think reviewed by should limit its scope to code correctness leaving the subjective worthwhile statements are better expressed with other tags. A patch which is not worthwhile is also not appropriate. Mere correctness in a mathematical sense is not enough as technical review criterion. Yes, but there's also such thing as worthwhile removal. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
J. Bruce Fields wrote: And it'd be nice to have a tag for those sorts of contributions, partly just as another way to ackowledge them. Yes, although the primary purpose of the various tags should be to document the quality assurance process, or how to call it. However, what belongs into the SCM changelog, and what can the list archives provide? -- Stefan Richter -=-=-=== =-=- -=--- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Al Viro [EMAIL PROTECTED] wrote: A patch which is not worthwhile is also not appropriate. Mere correctness in a mathematical sense is not enough as technical review criterion. Yes, but there's also such thing as worthwhile removal. Good point. So the text should probably say worthwhile change rather than worthwhile addition. I do believe that thinking about whether the change as a whole is a desirable thing is an important part of the review process. jon - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, J. Bruce Fields [EMAIL PROTECTED] wrote: On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: ... So, putting a Tested-by into the changelog is only useful if the necessary testing is rather simple (i.e. fixed the bug which I was always able to reproduce before) or if the tester is known to have performed rigorous and sufficiently broad tests. Well, you can still include those test-method details in the body of the message in addition to adding the Tested-by:. Does Tested-by just mean they ran some relevant test on the final version of the patch? The really hard part is often the initial work required to find a good reproduceable test case, capture the right error report, or bisect to the right commit. I think that also counts as testing. And it'd be nice to have a tag for those sorts of contributions, partly just as another way to ackowledge them. --- Tested-by should, at the very least, have a description of the test environment in the body (suggesting that the change compiled and ran in that environment). Preferably it should also have a description of the test or test suite run and whether that test failed on an unpatched system. scott -- scott preece - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On 10/8/07, Jan Engelhardt [EMAIL PROTECTED] wrote: On Oct 8 2007 19:37, Sam Ravnborg wrote: snip... Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth --- A couple of months ago there was a thread about Acked-by. At that time the consensus seemed to be that it was inappropriate for random people to add Acked-by - that it should only be added by people whose opinion was expected to a gate for merging. Did the Summit reach a decision that Acked-by was for anybody and Reviewed-by was for authorities? I like Randy's point - the assigning of weight to the Reviewer's opinion is necessarily in the hands of those at the top of the merge process and there's no need to ask reviewers to decide whether their opinion matters. In that view, Acked-by means I have no objection to this patch, but don't claim deep review and Reviewed-by means I have no objection to this patch after a thorough review, and either can be attached by anybody who wants to express an opinion. scott -- scott preece - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday, 8 October 2007 21:26, Scott Preece wrote: On 10/8/07, J. Bruce Fields [EMAIL PROTECTED] wrote: On Mon, Oct 08, 2007 at 08:34:47PM +0200, Stefan Richter wrote: ... So, putting a Tested-by into the changelog is only useful if the necessary testing is rather simple (i.e. fixed the bug which I was always able to reproduce before) or if the tester is known to have performed rigorous and sufficiently broad tests. Well, you can still include those test-method details in the body of the message in addition to adding the Tested-by:. Does Tested-by just mean they ran some relevant test on the final version of the patch? The really hard part is often the initial work required to find a good reproduceable test case, capture the right error report, or bisect to the right commit. I think that also counts as testing. And it'd be nice to have a tag for those sorts of contributions, partly just as another way to ackowledge them. --- Tested-by should, at the very least, have a description of the test environment in the body (suggesting that the change compiled and ran in that environment). Preferably it should also have a description of the test or test suite run and whether that test failed on an unpatched system. Tested-by: is sort of trivial for a fix patch, for example, if a bug reporter confirms that the proposed patch actually fixes the issue. IMHO it wouldn't be practical to complicate that. Greetings, Rafael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jan Engelhardt wrote: Acked-by: Tested-by: * Used by random people to express their (dis)like/experience with the patch. Reviewed-by: * I am maintaner or an 'important' person and have had a look at it in depth Uhm, no. There is no reason an unimportant person couldn't review a patch, and therefore perform a potentially highly valuable service to the maintainer. None of these are indicative of the authority of the person acking, reviewing, testing, or nacking. That's only as good as the trust in the person signing. -hpa - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: Uhm, no. There is no reason an unimportant person couldn't review a patch, and therefore perform a potentially highly valuable service to the maintainer. None of these are indicative of the authority of the person acking, reviewing, testing, or nacking. That's only as good as the trust in the person signing. I would tend to agree. Right now I think the problem is that we are getting too little reviews, not enough. And someone who reviews patches, even if unknown, could be building up expertise that eventually would make them a valued developer, even while they are doing us a service. The concern that I suspect some people have is what if this gets abused by people who don't really bother to do a full review of a patch before they ack it. We could ask reviewers to include a URL to an LKML archive of their review, to make it easier to find a review of a patch so later on people can judge how effective they their review was. Unfortunately, this would be an added burden for the regular reviewers, so I doubt this would be well accepted as a practice. My suggestion is to not worry about this for now, and see how well it works out in practice. If we start getting half a dozen or more Reviewed-by: where the patch is pretty clearly not getting adequately reviewed, or where someone is obviously abusing the system, and social pressures aren't working, we can try to figure out then how we want to address that problem then. Let's not make the process too complicated unless we know it's necessary. Premature complexity is almost as bad as premature optimization - Ted - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday, 8 October 2007 23:38, Theodore Tso wrote: On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: Uhm, no. There is no reason an unimportant person couldn't review a patch, and therefore perform a potentially highly valuable service to the maintainer. None of these are indicative of the authority of the person acking, reviewing, testing, or nacking. That's only as good as the trust in the person signing. I would tend to agree. Right now I think the problem is that we are getting too little reviews, not enough. And someone who reviews patches, even if unknown, could be building up expertise that eventually would make them a valued developer, even while they are doing us a service. The concern that I suspect some people have is what if this gets abused by people who don't really bother to do a full review of a patch before they ack it. We could ask reviewers to include a URL to an LKML archive of their review, to make it easier to find a review of a patch so later on people can judge how effective they their review was. Unfortunately, this would be an added burden for the regular reviewers, so I doubt this would be well accepted as a practice. My suggestion is to not worry about this for now, and see how well it works out in practice. If we start getting half a dozen or more Reviewed-by: where the patch is pretty clearly not getting adequately reviewed, or where someone is obviously abusing the system, and social pressures aren't working, we can try to figure out then how we want to address that problem then. Let's not make the process too complicated unless we know it's necessary. Premature complexity is almost as bad as premature optimization I agree. Greetings, Rafael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Sam Ravnborg [EMAIL PROTECTED] wrote: Or maybe we need something much less formal that explain the purpose of the four tags we use: ...or maybe a combination? How does the following patch look as a way to describe how the tags are used and what Reviewed-by, in particular, means? Perhaps the DCO should move to this file as well? jon --- Add a document on patch tags. Signed-off-by: Jonathan Corbet [EMAIL PROTECTED] diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX index 43e89b1..fa1518b 100644 --- a/Documentation/00-INDEX +++ b/Documentation/00-INDEX @@ -284,6 +284,8 @@ parport.txt - how to use the parallel-port driver. parport-lowlevel.txt - description and usage of the low level parallel port functions. +patch-tags + - description of the tags which can be added to patches pci-error-recovery.txt - info on PCI error recovery. pci.txt diff --git a/Documentation/patch-tags b/Documentation/patch-tags new file mode 100644 index 000..fb5f8e1 --- /dev/null +++ b/Documentation/patch-tags @@ -0,0 +1,66 @@ +Patches headed for the mainline may contain a variety of tags documenting +who played a hand in (or was at least aware of) its progress. All of these +tags have the form: + + Something-done-by: Full name [EMAIL PROTECTED] + +These tags are: + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. + +Acked-by: The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. + +Reviewed-by: The patch has been reviewed and found acceptible according + to the Reviewer's Statement as found at the bottom of this + file. A Reviewed-by tag is a statement of opinion that the + patch is an appropriate modification of the kernel without + any remaining serious technical issues. Any interested + reviewer (who has done the work) can offer a Reviewed-by + tag for a patch. + +Cc:The person named was given the opportunity to comment on + the patch. This is the only tag which might be added + without an explicit action by the person it names. + +Tested-by: The patch has been successfully tested (in some + environment) by the person named. + + + + +Reviewer's statement of oversight, v0.02 + +By offering my Reviewed-by: tag, I state that: + + (a) I have carried out a technical review of this patch to evaluate its + appropriateness and readiness for inclusion into the mainline kernel. + + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. + + (c) While there may (or may not) be things which could be improved with + this submission, I believe that it is, at this time, (1) a worthwhile + modification to the kernel, and (2) free of known issues which would + argue against its inclusion. + + (d) While I have reviewed the patch and believe it to be sound, I can not + (unless explicitly stated elsewhere) make any warranties or guarantees + that it will achieve its stated purpose or function properly in any + given situation. + + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
* Mon, 8 Oct 2007 17:38:52 -0400 On Mon, Oct 08, 2007 at 01:33:38PM -0700, H. Peter Anvin wrote: Uhm, no. There is no reason an unimportant person couldn't review a patch, and therefore perform a potentially highly valuable service to the maintainer. None of these are indicative of the authority of the person acking, reviewing, testing, or nacking. That's only as good as the trust in the person signing. I would tend to agree. Right now I think the problem is that we are getting too little reviews, not enough. And someone who reviews patches, even if unknown, could be building up expertise that eventually would make them a valued developer, even while they are doing us a service. Experience of convincing experienced patch author, that some things in the patch are wrong :) [] We could ask reviewers to include a URL to an LKML archive of their review, to make it easier to find a review of a patch so later on people can judge how effective they their review was. I vote for more little summaries in the `Subject'(again). Long, boring threads with whole threading part of screen being empty due to same subjects isn't fun, when some of thousands of messages can have interesting stuff inside. And it's easy not only for mailing list readers now, and for archive readers also; readers of the www search results (who ever that may be): google.com/search?q=reviewed+crashkernel First hit on the review of the patch, i happened to make. And i just thought hell, just string parsing, what can be more simply?, yet there was productive discussion and bug fixing. After i saw convincing statements about testing, i've placed review mark. Though i'm really unimportant random hacker. -- -o--=O`C #oo'L O ___=E M - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 08 Oct 2007 16:43:10 -0600 Jonathan Corbet wrote: Sam Ravnborg [EMAIL PROTECTED] wrote: Or maybe we need something much less formal that explain the purpose of the four tags we use: ...or maybe a combination? How does the following patch look as a way to describe how the tags are used and what Reviewed-by, in particular, means? Perhaps the DCO should move to this file as well? jon Just typos noted below... --- Add a document on patch tags. Signed-off-by: Jonathan Corbet [EMAIL PROTECTED] diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX index 43e89b1..fa1518b 100644 --- a/Documentation/00-INDEX +++ b/Documentation/00-INDEX @@ -284,6 +284,8 @@ parport.txt - how to use the parallel-port driver. parport-lowlevel.txt - description and usage of the low level parallel port functions. +patch-tags + - description of the tags which can be added to patches pci-error-recovery.txt - info on PCI error recovery. pci.txt diff --git a/Documentation/patch-tags b/Documentation/patch-tags new file mode 100644 index 000..fb5f8e1 --- /dev/null +++ b/Documentation/patch-tags @@ -0,0 +1,66 @@ +Patches headed for the mainline may contain a variety of tags documenting +who played a hand in (or was at least aware of) its progress. All of these +tags have the form: + + Something-done-by: Full name [EMAIL PROTECTED] + +These tags are: + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. + +Acked-by:The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. + +Reviewed-by: The patch has been reviewed and found acceptible according acceptable + to the Reviewer's Statement as found at the bottom of this + file. A Reviewed-by tag is a statement of opinion that the + patch is an appropriate modification of the kernel without + any remaining serious technical issues. Any interested + reviewer (who has done the work) can offer a Reviewed-by + tag for a patch. + +Cc: The person named was given the opportunity to comment on + the patch. This is the only tag which might be added + without an explicit action by the person it names. + +Tested-by: The patch has been successfully tested (in some + environment) by the person named. + + + + +Reviewer's statement of oversight, v0.02 + +By offering my Reviewed-by: tag, I state that: + + (a) I have carried out a technical review of this patch to evaluate its + appropriateness and readiness for inclusion into the mainline kernel. + + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. + + (c) While there may (or may not) be things which could be improved with + this submission, I believe that it is, at this time, (1) a worthwhile + modification to the kernel, and (2) free of known issues which would + argue against its inclusion. + + (d) While I have reviewed the patch and believe it to be sound, I can not cannot + (unless explicitly stated elsewhere) make any warranties or guarantees + that it will achieve its stated purpose or function properly in any + given situation. + + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. - --- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 04:43:10PM -0600, Jonathan Corbet wrote: + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent with this project or + the open source license(s) involved. Is this paragraph really necessary? (For example, is there some history of problems that this is addressing?) --b. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
Jonathan Corbet wrote: All of these +tags have the form: + + Something-done-by: Full name [EMAIL PROTECTED] To be precise: Something-done-by: Full name [EMAIL PROTECTED] [optional random stuff] Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off., says SubmittingPatches. I actually do so on occasions. -- Stefan Richter -=-=-=== =-=- -=--= http://arcgraph.de/sr/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Monday October 8, [EMAIL PROTECTED] wrote: I find it is always good to know *why* we have the tags. That information is a useful complement to what they mean, and can guide people in adding them. So below I present some Purposes, YetAnotherTag, and a comment on the RSO. (And I'd like to add a vote for Blame-Shared-By: rather than Reviewed-by:, however I don't I'll get much support...) diff --git a/Documentation/patch-tags b/Documentation/patch-tags new file mode 100644 index 000..fb5f8e1 --- /dev/null +++ b/Documentation/patch-tags @@ -0,0 +1,66 @@ +Patches headed for the mainline may contain a variety of tags documenting +who played a hand in (or was at least aware of) its progress. All of these +tags have the form: + + Something-done-by: Full name [EMAIL PROTECTED] + +These tags are: From:The Author, Primary Author, or Authors of the patch. Authors should also provide a Signed-off-by: tag. Purpose: to give credit to authors + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. Purpose: to allow subsequent review of the originality of the contribution should copyright questions arise. + +Acked-by:The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. Purpose: to inform upstream aggregators that consensus was achieved for the change. This is particularly relevant for changes that affect multiple Maintenance Domains. + +Reviewed-by: The patch has been reviewed and found acceptible according + to the Reviewer's Statement as found at the bottom of this + file. A Reviewed-by tag is a statement of opinion that the + patch is an appropriate modification of the kernel without + any remaining serious technical issues. Any interested + reviewer (who has done the work) can offer a Reviewed-by + tag for a patch. Purpose: to inform upstream aggregators that due diligence has been performed to ensure correctness of the change. Also to give credit to reviewers. + +Cc: The person named was given the opportunity to comment on + the patch. This is the only tag which might be added + without an explicit action by the person it names. Purpose: to ensure that interested parties are included in subsequent discussions of the change. + +Tested-by: The patch has been successfully tested (in some + environment) by the person named. Purpose: to give credit to testers. + + + + +Reviewer's statement of oversight, v0.02 + +By offering my Reviewed-by: tag, I state that: + + (a) I have carried out a technical review of this patch to evaluate its + appropriateness and readiness for inclusion into the mainline kernel. + + (b) Any problems, concerns, or questions relating to the patch have been + communicated back to the submitter. I am satisfied with how the + submitter has responded to my comments. This seems more detailed that necessary. The process (communicated back / responded) is not really relevant. I would go for something like: (b) I have no outstanding problems, concerns, or questions about this patch (except as noted in the above comments). and in fact, given (c2), (b) might not be needed at all. NeilBrown + + (c) While there may (or may not) be things which could be improved with + this submission, I believe that it is, at this time, (1) a worthwhile + modification to the kernel, and (2) free of known issues which would + argue against its inclusion. + + (d) While I have reviewed the patch and believe it to be sound, I can not + (unless explicitly stated elsewhere) make any warranties or guarantees + that it will achieve its stated purpose or function properly in any + given situation. + + (e) I understand and agree that this project and the contribution are + public and that a record of the contribution (including my Reviewed-by + tag and any associated public communications) is maintained + indefinitely and may be redistributed consistent
Re: RFC: reviewer's statement of oversight
On Mon, Oct 08, 2007 at 10:16:26PM +0200, Rafael J. Wysocki wrote: Tested-by: is sort of trivial for a fix patch, for example, if a bug reporter confirms that the proposed patch actually fixes the issue. IMHO it wouldn't be practical to complicate that. I see two types of Tested-by. 1) As you stated, a fixed to a problem that the reporter has seen. So that someone could state a fixes issue in the change log and that would simple mean that the tester has seen a problem, and the attached patch fixes it. 2) Someone has a testsuite to the area that the change affects. So if someone has developed a networking test suite and a patch changes some networking logic, the Tested-by could be that the tester actually ran specific tests. This should require a more detail explaination of what was done. Or the very least, a pointer to a web page of the tests that were run. So for the user that sees an issue, then gets a patch, perhaps all they need to do is add a fixed problem or works now in the change log to denote that the patch has actually (or seems to) fix the problem that they previously seen. This shouldn't be too hard. But for those that run test suites, they should be smart enough to put in more documentation into the change log to state how it was tested. Perhaps we need to add yet another signed off. Verified-by, which could be for the user that saw an issue and the patch now fixes it. That user could just add the Verified-by to the patch to acknowledge (and record) that the patch did fix the issue. The Tested-by can be used for patches that are run through a test suite. Just a thought. -- Steve - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: RFC: reviewer's statement of oversight
On Mon, 8 Oct 2007 16:06:03 -0700 Randy Dunlap [EMAIL PROTECTED] wrote: On Mon, 08 Oct 2007 16:43:10 -0600 Jonathan Corbet wrote: Sam Ravnborg [EMAIL PROTECTED] wrote: Or maybe we need something much less formal that explain the purpose of the four tags we use: ...or maybe a combination? How does the following patch look as a way to describe how the tags are used and what Reviewed-by, in particular, means? Perhaps the DCO should move to this file as well? jon Just typos noted below... --- Add a document on patch tags. Signed-off-by: Jonathan Corbet [EMAIL PROTECTED] diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX index 43e89b1..fa1518b 100644 --- a/Documentation/00-INDEX +++ b/Documentation/00-INDEX @@ -284,6 +284,8 @@ parport.txt - how to use the parallel-port driver. parport-lowlevel.txt - description and usage of the low level parallel port functions. +patch-tags + - description of the tags which can be added to patches pci-error-recovery.txt - info on PCI error recovery. pci.txt diff --git a/Documentation/patch-tags b/Documentation/patch-tags new file mode 100644 index 000..fb5f8e1 --- /dev/null +++ b/Documentation/patch-tags @@ -0,0 +1,66 @@ +Patches headed for the mainline may contain a variety of tags documenting +who played a hand in (or was at least aware of) its progress. All of these +tags have the form: + + Something-done-by: Full name [EMAIL PROTECTED] + +These tags are: + +Signed-off-by: A person adding a Signed-off-by tag is attesting that the + patch is, to the best of his or her knowledge, legally able + to be merged into the mainline and distributed under the + terms of the GNU General Public License, version 2. All changes are licensed under the terms of the file modified. (Some people seem not to understand that if the file is dual licensed, then the changes are dual licensed. If file is GPL v2 only, then the changes are GPL v2 only, ...) See + the Developer's Certificate of Origin, found in + Documentation/SubmittingPatches, for the precise meaning of + Signed-off-by. +Acked-by: The person named (who should be an active developer in the + area addressed by the patch) is aware of the patch and has + no objection to its inclusion. An Acked-by tag does not + imply any involvement in the development of the patch or + that a detailed review was done. + +Reviewed-by: The patch has been reviewed and found acceptible according acceptable + to the Reviewer's Statement as found at the bottom of this + file. A Reviewed-by tag is a statement of opinion that the + patch is an appropriate modification of the kernel without + any remaining serious technical issues. Any interested + reviewer (who has done the work) can offer a Reviewed-by + tag for a patch. + +Cc:The person named was given the opportunity to comment on + the patch. This is the only tag which might be added + without an explicit action by the person it names. + +Tested-by: The patch has been successfully tested (in some + environment) by the person named. + IMHO the other tags actually are a poor substitute for providing a more complete description of the reviewer's involvement. It would be better to have more complete responses like the patch should be merged as is for 2.6.X but the following should be fixed, ... etc. The certificate of origin has meaning for legal things that have a more concrete definition, but the existing process is about people making good (or bad) decisions based on feedback and other data. Trying to reduce the feedback down to 3 Acks, and 1 Review seems like noise. The problem is getting good reviews of new code in a timely manner, not the descriptions of the result. -- Stephen Hemminger [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
