Somebody recently asked me about this patch, so I dug it up for one last
try. I do believe there is value in describing patch tags, and,
certainly, nobody has objected to the idea. Comments from several
reviewers were addressed before the previous posting.
jon
--
Add a document describing the various tags attached to patches.
Signed-off-by: Jonathan Corbet <[email protected]>
---
Documentation/00-INDEX | 2 +
Documentation/patch-tags | 76 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 0 deletions(-)
create mode 100644 Documentation/patch-tags
diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 6e9c405..a900a6d 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -289,6 +289,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 0000000..c2fb56c
--- /dev/null
+++ b/Documentation/patch-tags
@@ -0,0 +1,76 @@
+Patches headed for the mainline may contain a variety of tags documenting
+who played a hand in (or was at least aware of) their progress. All of
+these tags have the form:
+
+ Something-done-by: Full name <email@address> [optional random stuff]
+
+These tags are:
+
+From: The original author of the patch. This tag will ensure
+ that credit is properly given when somebody other than the
+ original author submits the patch.
+
+Signed-off-by: A person adding a Signed-off-by tag is attesting that the
+ patch, to the best of his or her knowledge, can legally 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. This tag assures upstream maintainers that
+ the provenance of the patch is known and allows the origin
+ of the patch to be reviewed 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; it informs upstream
+ maintainers that a certain degree of consensus on the patch
+ as been achieved.. 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 acceptable 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. This tag serves to give credit to
+ reviewers and to inform maintainers of the degree of review
+ which has been done on the 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. This
+ tag documents that potentially interested parties have been
+ included in the discussion.
+
+Tested-by: The patch has been successfully tested (in some
+ environment) by the person named. This tag informs
+ maintainers that some testing has been performed, provides
+ a means to locate testers for future patches, and ensures
+ credit for the testers.
+
+
+----
+
+Reviewer's statement of oversight
+
+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 the
+ submitter's response to my comments.
+
+ (c) While there may be things that 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 do not
+ (unless explicitly stated elsewhere) make any warranties or guarantees
+ that it will achieve its stated purpose or function properly in any
+ given situation.
--
1.5.4
Hi,
On Feb 8, 2008 6:51 PM, Jonathan Corbet <[email protected]> wrote:
> Somebody recently asked me about this patch, so I dug it up for one last
> try. I do believe there is value in describing patch tags, and,
> certainly, nobody has objected to the idea. Comments from several
> reviewers were addressed before the previous posting.
Andrew, can we please get this patch merged? People seem to be abusing
"Signed-off-by" for these things so it would be nice to have it
documented in the official kernel tree. Thanks.
Pekka
On Fri, 8 Feb 2008, Jonathan Corbet wrote:
> +
> +These tags are:
> +
> +From: The original author of the patch. This tag will ensure
> + that credit is properly given when somebody other than the
> + original author submits the patch.
"From:" is not a tag. It's a special marker at the *top* (and _only_ the
top) of the email, which indicates authorship. It goes along with markers
like "Date:" and "Subject:", and has nothing to do with the sign-off-like
tags at the end.
Linus
Linus wrote:
> "From:" is not a tag. It's a special marker at the *top* (and _only_ the top)
So ... instead of listing "From:" as just another tag, I'd think it would
work if this one was listed in a separate section, perhaps at the end of this
Documentation/patch-tags document, such as:
From: In addition to the above tags, one can also specify the original author
of the change, who will end up as the commit author, with a special marker
at the top (very first line) of the patch, with a "From:" line. See further
Documentation/SubmittingPatches for usage of this "From:" line.
Question -- should this documentation of patch-tags be in its own file,
or added to Documentation/SubmittingPatches. If it remains in its own
file, then perhaps Documentation/SubmittingPatches should have a reference
to Documentation/patch-tags added.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214
OK, Linus questioned the From: tag, so I've just taken that out for
now. Paul Jackson asked:
> Question -- should this documentation of patch-tags be in its own file,
> or added to Documentation/SubmittingPatches.
Clearly I had once thought the former, but, on review, I've changed my
mind. So here's a version which merges the information into
SubmittingPatches instead.
Thanks,
jon
--
Add documentation for more patch tags
Add documentation of the Cc:, Tested-by:, and Reviewed-by: tags to
SubmittingPatches, with an emphasis on trying to nail down what
Reviewed-by: really means.
Signed-off-by: Jonathan Corbet <[email protected]>
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 08a1ed1..cc00c8e 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -328,7 +328,7 @@ now, but you can do this to mark internal company procedures or just
point out some special detail about the sign-off.
-13) When to use Acked-by:
+13) When to use Acked-by: and Cc:
The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.
@@ -349,11 +349,59 @@ Acked-by: does not necessarily indicate acknowledgement of the entire patch.
For example, if a patch affects multiple subsystems and has an Acked-by: from
one subsystem maintainer then this usually indicates acknowledgement of just
the part which affects that maintainer's code. Judgement should be used here.
- When in doubt people should refer to the original discussion in the mailing
+When in doubt people should refer to the original discussion in the mailing
list archives.
+If a person has had the opportunity to comment on a patch, but has not
+provided such comments, you may optionally add a "Cc:" tag to the patch.
+This is the only tag which might be added without an explicit action by the
+person it names. This tag documents that potentially interested parties
+have been included in the discussion
-14) The canonical patch format
+
+14) Using Test-by: and Reviewed-by:
+
+A Tested-by: tag indicates that the patch has been successfully tested (in
+some environment) by the person named. This tag informs maintainers that
+some testing has been performed, provides a means to locate testers for
+future patches, and ensures credit for the testers.
+
+Reviewed-by:, instead, indicates that the patch has been reviewed and found
+acceptable according to the Reviewer's Statement:
+
+ Reviewer's statement of oversight
+
+ 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 the submitter's response to my comments.
+
+ (c) While there may be things that 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
+ do not (unless explicitly stated elsewhere) make any
+ warranties or guarantees that it will achieve its stated
+ purpose or function properly in any given situation.
+
+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. This tag serves to give credit to
+reviewers and to inform maintainers of the degree of review which has been
+done on the patch. Reviewed-by: tags, when supplied by reviewers known to
+understand the subject area and to perform thorough reviews, will normally
+increase the liklihood of your patch getting into the kernel.
+
+
+15) The canonical patch format
The canonical patch subject line is:
Linus Torvalds <[email protected]> writes:
> On Fri, 8 Feb 2008, Jonathan Corbet wrote:
>> +
>> +These tags are:
>> +
>> +From: The original author of the patch. This tag will ensure
>> + that credit is properly given when somebody other than the
>> + original author submits the patch.
>
> "From:" is not a tag. It's a special marker at the *top* (and _only_ the
> top) of the email, which indicates authorship. It goes along with markers
> like "Date:" and "Subject:", and has nothing to do with the sign-off-like
> tags at the end.
True. When one or more of these appear at the top (and _only_
the top) of a patch e-mail,
From: the person who wrote the patch
Date: the date of the authorship
Subject: the title of the patch
we override the corresponding values we obtain from the mail
header. Documenting them is probably a good idea but they
should be described separately from S-o-b: and friends to avoid
confusion.
Jon wrote:
> So here's a version which merges the information into
> SubmittingPatches instead.
Reviewed-by: Paul Jackson <[email protected]>
---
That's my first time using that tag ... fun. Now I'm wondering if there
might be some improvements that you, me, or someone could make to the
SubmittingPatches file that would explain more clearly that PATCH's
should start off, right from the top or after the initial From: line if
present, with:
- The body of the explanation, which will be copied to the
permanent changelog to describe this patch.
rather than opening with ephemeral reactions to the discussion of the
moment which will make little "sense to a competent reader who has long
since forgotten the immediate details of the discussion that might have
led to this patch."
As best my legalistic mind understands the patch format, such ephemeral
reactions should go after the "---" marker line, in the same section as
the diffstat would be placed.
Or perhaps the patch format should be changed in this regard, since it
is cumbersome and unnatural to embed the ephemeral opening comments
half way down the patch, rather than opening with them. I don't know.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214
Paul Jackson would like to:
> explain more clearly that PATCH's
> should start off, right from the top or after the initial From: line if
> present, with:
>
> - The body of the explanation, which will be copied to the
> permanent changelog to describe this patch.
>
> rather than opening with ephemeral reactions to the discussion of the
> moment which will make little "sense to a competent reader who has long
> since forgotten the immediate details of the discussion that might have
> led to this patch."
>
> As best my legalistic mind understands the patch format, such ephemeral
> reactions should go after the "---" marker line, in the same section as
> the diffstat would be placed.
>
> Or perhaps the patch format should be changed in this regard, since it
> is cumbersome and unnatural to embed the ephemeral opening comments
> half way down the patch, rather than opening with them.
In common practice, the changelog does not start before the _last_ From:
line (rather than the initial From: line).
ephemera
From: author
changelog
tags
changelog, continued
tags
---
ephemera, diffstat
diff
ephemera
There may be a Date: somewhere before the changelog as well. The first
line of the changelog is the one to appear in shortlogs. It may be
preceded by Subject:.
--
Stefan Richter
-=====-==--- --=- -=--=
http://arcgraph.de/sr/
On Friday February 8, [email protected] wrote:
>
>
> On Fri, 8 Feb 2008, Jonathan Corbet wrote:
> > +
> > +These tags are:
> > +
> > +From: The original author of the patch. This tag will ensure
> > + that credit is properly given when somebody other than the
> > + original author submits the patch.
>
> "From:" is not a tag. It's a special marker at the *top* (and _only_ the
> top) of the email, which indicates authorship. It goes along with markers
> like "Date:" and "Subject:", and has nothing to do with the sign-off-like
> tags at the end.
You may be right, but when I email patches to akpm that I did not
author, and that don't have a From: tag in the body saying who did
author them, then akpm tells me off. And I don't like that :-(
Maybe we need "tags in git commit logs" and "tags in email sent to
akpm" and ... :-)
NeilBrown
Neil responding to Linus:
> > "From:" is not a tag. It's a special marker at the *top*
>
> You may be right, but when I email patches to akpm
Linus wasn't saying you don't need a 'From:' line in this case (as the
*top* line of patches you didn't author). He's saying it's not an
instance of the type "tag" line, such as the "Signed-off-by:" line that
go after the patch explanation. The "From:" line is a different kind
of line, with different rules and position.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.940.382.4214
On Sunday February 17, [email protected] wrote:
> Neil responding to Linus:
> > > "From:" is not a tag. It's a special marker at the *top*
> >
> > You may be right, but when I email patches to akpm
>
> Linus wasn't saying you don't need a 'From:' line in this case (as the
> *top* line of patches you didn't author). He's saying it's not an
> instance of the type "tag" line, such as the "Signed-off-by:" line that
> go after the patch explanation. The "From:" line is a different kind
> of line, with different rules and position.
If it looks like a duck and quacks like a duck.....
When Linus used "From" "Subject" and "Date" together in the context of
"mail", it sounded a lot like he was talking about the headers section
of a standard mail item. I guess he wasn't.
I don't really care if "From:" gets called a 'tag' or what, but given
that it is an important piece of metadata associated with a patch, it
seems good to document it in the same place as other important pieces
of metadata that are associated with patches.
NeilBrown