LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Documentation/patch-tags, one more time
@ 2008-02-08 16:51 Jonathan Corbet
  2008-02-08 17:07 ` Pekka Enberg
  2008-02-08 18:00 ` Linus Torvalds
  0 siblings, 2 replies; 11+ messages in thread
From: Jonathan Corbet @ 2008-02-08 16:51 UTC (permalink / raw)
  To: linux-kernel; +Cc: akpm, torvalds

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 <corbet@lwn.net>
---
 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-08 16:51 [PATCH] Documentation/patch-tags, one more time Jonathan Corbet
@ 2008-02-08 17:07 ` Pekka Enberg
  2008-02-08 18:00 ` Linus Torvalds
  1 sibling, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2008-02-08 17:07 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-kernel, akpm, torvalds

Hi,

On Feb 8, 2008 6:51 PM, Jonathan Corbet <corbet@lwn.net> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-08 16:51 [PATCH] Documentation/patch-tags, one more time Jonathan Corbet
  2008-02-08 17:07 ` Pekka Enberg
@ 2008-02-08 18:00 ` Linus Torvalds
  2008-02-08 19:01   ` Paul Jackson
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Linus Torvalds @ 2008-02-08 18:00 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-kernel, akpm



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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-08 18:00 ` Linus Torvalds
@ 2008-02-08 19:01   ` Paul Jackson
  2008-02-08 20:42     ` [PATCH] Documenting patch tags yet " Jonathan Corbet
  2008-02-08 20:50   ` [PATCH] Documentation/patch-tags, " Junio C Hamano
  2008-02-18  2:58   ` Neil Brown
  2 siblings, 1 reply; 11+ messages in thread
From: Paul Jackson @ 2008-02-08 19:01 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: corbet, linux-kernel, akpm

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 <pj@sgi.com> 1.940.382.4214

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Documenting patch tags yet one more time
  2008-02-08 19:01   ` Paul Jackson
@ 2008-02-08 20:42     ` Jonathan Corbet
  2008-02-09  0:23       ` Paul Jackson
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Corbet @ 2008-02-08 20:42 UTC (permalink / raw)
  To: Paul Jackson; +Cc: Linus Torvalds, linux-kernel, akpm

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 <corbet@lwn.net>

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:
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-08 18:00 ` Linus Torvalds
  2008-02-08 19:01   ` Paul Jackson
@ 2008-02-08 20:50   ` Junio C Hamano
  2008-02-18  2:58   ` Neil Brown
  2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2008-02-08 20:50 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: corbet, linux-kernel, akpm

Linus Torvalds <torvalds@linux-foundation.org> 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.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documenting patch tags yet one more time
  2008-02-08 20:42     ` [PATCH] Documenting patch tags yet " Jonathan Corbet
@ 2008-02-09  0:23       ` Paul Jackson
  2008-02-09  8:37         ` Stefan Richter
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Jackson @ 2008-02-09  0:23 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: torvalds, linux-kernel, akpm

Jon wrote:
> So here's a version which merges the information into
> SubmittingPatches instead.

Reviewed-by: Paul Jackson <pj@sgi.com>

---

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 <pj@sgi.com> 1.940.382.4214

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documenting patch tags yet one more time
  2008-02-09  0:23       ` Paul Jackson
@ 2008-02-09  8:37         ` Stefan Richter
  0 siblings, 0 replies; 11+ messages in thread
From: Stefan Richter @ 2008-02-09  8:37 UTC (permalink / raw)
  To: Paul Jackson; +Cc: Jonathan Corbet, torvalds, linux-kernel, akpm

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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-08 18:00 ` Linus Torvalds
  2008-02-08 19:01   ` Paul Jackson
  2008-02-08 20:50   ` [PATCH] Documentation/patch-tags, " Junio C Hamano
@ 2008-02-18  2:58   ` Neil Brown
  2008-02-18  3:15     ` Paul Jackson
  2 siblings, 1 reply; 11+ messages in thread
From: Neil Brown @ 2008-02-18  2:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jonathan Corbet, linux-kernel, akpm

On Friday February 8, torvalds@linux-foundation.org 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-18  2:58   ` Neil Brown
@ 2008-02-18  3:15     ` Paul Jackson
  2008-02-18  3:58       ` Neil Brown
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Jackson @ 2008-02-18  3:15 UTC (permalink / raw)
  To: Neil Brown; +Cc: torvalds, corbet, linux-kernel, akpm

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 <pj@sgi.com> 1.940.382.4214

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Documentation/patch-tags, one more time
  2008-02-18  3:15     ` Paul Jackson
@ 2008-02-18  3:58       ` Neil Brown
  0 siblings, 0 replies; 11+ messages in thread
From: Neil Brown @ 2008-02-18  3:58 UTC (permalink / raw)
  To: Paul Jackson; +Cc: torvalds, corbet, linux-kernel, akpm

On Sunday February 17, pj@sgi.com 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-02-18  3:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-08 16:51 [PATCH] Documentation/patch-tags, one more time Jonathan Corbet
2008-02-08 17:07 ` Pekka Enberg
2008-02-08 18:00 ` Linus Torvalds
2008-02-08 19:01   ` Paul Jackson
2008-02-08 20:42     ` [PATCH] Documenting patch tags yet " Jonathan Corbet
2008-02-09  0:23       ` Paul Jackson
2008-02-09  8:37         ` Stefan Richter
2008-02-08 20:50   ` [PATCH] Documentation/patch-tags, " Junio C Hamano
2008-02-18  2:58   ` Neil Brown
2008-02-18  3:15     ` Paul Jackson
2008-02-18  3:58       ` Neil Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).