[PATCH] Document patch subject line better

[PATCH] Document patch subject line better

[Paul Jackson]

Improve explanation of the Subject line fields in
Documentation/SubmittingPatches Canonical Patch Format.

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

Index: 2.6.14-rc2-mm2/Documentation/SubmittingPatches
===================================================================
--- 2.6.14-rc2-mm2.orig/Documentation/SubmittingPatches
+++ 2.6.14-rc2-mm2/Documentation/SubmittingPatches
@@ -305,7 +305,7 @@ point out some special detail about the 
 
 The canonical patch subject line is:
 
-    Subject: [PATCH 001/123] [<area>:] <explanation>
+    Subject: [PATCH 001/123] subsystem: summary phrase
 
 The canonical patch message body contains the following:
 
@@ -330,9 +330,25 @@ alphabetically by subject line - pretty 
 support that - since because the sequence number is zero-padded,
 the numerical and alphabetic sort is the same.
 
-See further details on how to phrase the "<explanation>" in the
-"Subject:" line in Andrew Morton's "The perfect patch", referenced
-below.
+The "subsystem" in the email's Subject should identify which
+area or subsystem of the kernel is being patched.
+
+The "summary phrase" in the email's Subject should concisely
+describe the patch which that email contains.  The "summary
+phrase" should not be a filename.  Do not use the same "summary
+phrase" for every patch in a whole patch series.
+
+Bear in mind that the "summary phrase" of your email becomes
+a globally-unique identifier for that patch.  It propagates
+all the way into the git changelog.  The "summary phrase" may
+later be used in developer discussions which refer to the patch.
+People will want to google for the "summary phrase" to read
+discussion regarding that patch.
+
+A couple of example Subjects:
+
+    Subject: [patch 2/5] ext2: improve scalability of bitmap searching
+    Subject: [PATCHv2 001/207] x86: fix eflags tracking
 
 The "from" line must be the very first line in the message body,
 and has the form:

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: [PATCH] Document patch subject line better

[Peter Zijlstra]

On Mon, 2005-10-03 at 00:29 -0700, Paul Jackson wrote:

> +The "summary phrase" in the email's Subject should concisely
> +describe the patch which that email contains.  The "summary
> +phrase" should not be a filename.  Do not use the same "summary
> +phrase" for every patch in a whole patch series.

On that last sentence, does quilt support having different subjects for
different patches?


-- 
Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH] Document patch subject line better

[Paul Jackson]

Peter wrote:
> On that last sentence, does quilt support having different subjects for
> different patches?

I'm missing a step in your thinking here - what does quilt
have to do with patch subjects?

I use quilt, but I don't use it to send patches.  To send
patches, I use sendpatchset:

  http://www.speakeasy.org/~pj99/sgi/sendpatchset

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [PATCH] Document patch subject line better

[Peter Zijlstra]

On Mon, 2005-10-03 at 01:42 -0700, Paul Jackson wrote:
> Peter wrote:
> > On that last sentence, does quilt support having different subjects for
> > different patches?
> 
> I'm missing a step in your thinking here - what does quilt
> have to do with patch subjects?
> 
> I use quilt, but I don't use it to send patches.  To send
> patches, I use sendpatchset:
> 
>   http://www.speakeasy.org/~pj99/sgi/sendpatchset
> 
Ah thanks for the pointer, but indeed, I do use quilt to send patches
and I expect quite a few people do so too.

-- 
Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH] Document patch subject line better

[Linus Torvalds]

On Mon, 3 Oct 2005, Paul Jackson wrote:
>
> Improve explanation of the Subject line fields in
> Documentation/SubmittingPatches Canonical Patch Format.

Ironically, the patch you sent doesn't adhere to the very documentation 
you are updating ;)

Hint: missing "---".

		Linus

Re: [PATCH] Document patch subject line better

[Paul Jackson]

Linus:
> Hint: missing "---".

True.

I did put it in, but "quilt refresh" took it out.

I send patches directly from my quilt patches directory.  The patches
file ends up being -exactly- the email message body.  I did put in a
"---", with a short comment about how this patch fit in with the
earlier ones of yesterday, and removed the "Index: " and "========="
lines that quilt adds.

But then I forgot and ran a "quilt refresh" before I sent the patch,
and it erased that "---" line and comments, and reestablished its
"Index: " and "=========" lines, as it always does.

Ideally either I should change my patch sending process, or I should
change quilt.  I don't know which yet.  Meanwhile, I am taking
advantage of the hack in your tools that filters out "Index: " lines.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [PATCH] Document patch subject line better

[Greg KH]

On Mon, Oct 03, 2005 at 08:54:14AM -0700, Paul Jackson wrote:
> 
> Ideally either I should change my patch sending process, or I should
> change quilt. 

Change quilt.  I have a horrible patch to my local copy of quilt that
adds this line, it's not hard to do.

I'll work on cleaning it up and getting the change into the upstream
version of quilt this week.

thanks,

greg k-h

Re: [PATCH] Document patch subject line better

[Jean Delvare]

Hi Paul,

> I send patches directly from my quilt patches directory.  The patches
> file ends up being -exactly- the email message body.  I did put in a
> "---", with a short comment about how this patch fit in with the
> earlier ones of yesterday, and removed the "Index: " and "========="
> lines that quilt adds.

FYI, quilt 0.41 and above have a --no-index option to the refresh
command, which will avoid this second annoyance. You can simply add the
following to ~/.quiltrc:

QUILT_NO_DIFF_INDEX=1

And the default behavior will be changed to match Linus' requirements.

-- 
Jean Delvare

Re: [PATCH] Document patch subject line better

[Greg KH]

On Mon, Oct 03, 2005 at 10:40:10PM +0200, Jean Delvare wrote:
> Hi Paul,
> 
> > I send patches directly from my quilt patches directory.  The patches
> > file ends up being -exactly- the email message body.  I did put in a
> > "---", with a short comment about how this patch fit in with the
> > earlier ones of yesterday, and removed the "Index: " and "========="
> > lines that quilt adds.
> 
> FYI, quilt 0.41 and above have a --no-index option to the refresh
> command, which will avoid this second annoyance. You can simply add the
> following to ~/.quiltrc:
> 
> QUILT_NO_DIFF_INDEX=1
> 
> And the default behavior will be changed to match Linus' requirements.

No, it still does not add the "---" line, right?

thanks,

greg k-h

Re: [PATCH] Document patch subject line better

[Jean Delvare]

Hi Greg, Paul,

> > Ideally either I should change my patch sending process, or I should
> > change quilt. 
> 
> Change quilt.  I have a horrible patch to my local copy of quilt that
> adds this line, it's not hard to do.
> 
> I'll work on cleaning it up and getting the change into the upstream
> version of quilt this week.

I wasn't aware of the problem, this clearly sounds like a bug to me. I
guess that picking "---" as a separator wasn't exactly a subtle choice,
but still...

The following patch fixes it for me:

Index: scripts/patchfns.in
===================================================================
RCS file: /cvsroot/quilt/quilt/scripts/patchfns.in,v
retrieving revision 1.75
diff -u -r1.75 patchfns.in
--- scripts/patchfns.in	18 Sep 2005 16:02:31 -0000	1.75
+++ scripts/patchfns.in	3 Oct 2005 20:50:23 -0000
@@ -612,7 +612,7 @@
 patch_header()
 {
 	awk '
-	$1 == "***" || $1 == "---" \
+	($1 == "***" || $1 == "---") && NF > 1 \
 		{ exit }
 	/^Index:[ \t]|^diff[ \t]|^==*$|^RCS file: |^retrieving revision [0-9]+(\.[0-9]+)*$/ \
 		{ eat = eat $0 "\n"
@@ -628,7 +628,7 @@
 	/^Index:[ \t]|^diff[ \t]|^==*$|^RCS file: |^retrieving revision [0-9]+(\.[0-9]+)*$/ \
 		{ eat = eat $0 "\n"
 		  next }
-	$1 == "***" || $1 == "---" \
+	($1 == "***" || $1 == "---") && NF > 1 \
 		{ body=1 }
 	body	{ print eat $0
 		  eat = ""


Comments?

This only prevents quilt from stripping the "---" line, it does NOT
add the line if it's not there. Doing so would require template
support, I know many users are interested and a few implementations
exist, waiting to be merged upstream, but it's not there right now.

Greg, if you have a better fix, just send it to the quilt-dev and I'll
get it applied.

-- 
Jean Delvare

Re: [PATCH] Document patch subject line better

[Greg KH]

On Mon, Oct 03, 2005 at 11:02:35PM +0200, Jean Delvare wrote:
> This only prevents quilt from stripping the "---" line, it does NOT
> add the line if it's not there. Doing so would require template
> support, I know many users are interested and a few implementations
> exist, waiting to be merged upstream, but it's not there right now.
> 
> Greg, if you have a better fix, just send it to the quilt-dev and I'll
> get it applied.

No, my fix just always added it.  I was just going to make that addition
optional based on a config option.  But your patch is also needed to
make sure it gets handled properly (I did something much like this, but
messier...)

thanks,

greg k-h

Re: [PATCH] Document patch subject line better

[Paul Jackson]

Jean wrote:
> This only prevents quilt from stripping the "---" line

This patch, plus the ~/.quiltrc option:

	QUILT_NO_DIFF_INDEX=1

do what I need.  Thanks for the quick response.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [PATCH] Document patch subject line better

[Paul Jackson]

Greg wrote (of the "---" line):
> No, my fix just always added it.

My preference would be to only add the "---" line
when starting with a brand new, empty patch file.

If an existing version of the patch file exists,
then just copy over the entire header (up to the
first actual patch) as is, no change.  If I choose
to edit the patch file header and remove the "---"
line, or do any other edit to it (short of creating
a line that looks like the start of a patch) then
quilt should respect that.

The following patch seems to accomplish this.

--- refresh.old	2005-10-03 17:31:41.000000000 -0700
+++ refresh	2005-10-03 20:02:04.000000000 -0700
@@ -247,7 +247,9 @@ fi
 
 mkdir -p $(dirname $patch_file)
 
-if ! cat_file $patch_file | patch_header > $tmp_header
+[ -s $patch_file ] || echo -e "\n---\n" > $tmp_header
+
+if ! cat_file $patch_file | patch_header >> $tmp_header
 then
 	die 1
 fi


-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401