Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jacob Keller]

On Mon, Sep 19, 2016 at 6:37 PM, Jeff King  wrote:
> On Mon, Sep 19, 2016 at 04:40:22PM -0700, Josh Triplett wrote:
>
>> >   - there are a non-trivial number of patches for other projects (JGIT,
>> > EGIT, StGit, etc). This is somewhat unique to git, where we discuss
>> > a lot of related projects on the list. But I wonder if other
>> > projects would use subsystems in a similar way (though I guess for
>> > the kernel, there are separate subsystems lists, so the "to" or "cc"
>> > header becomes the more interesting tag).
>>
>> The kernel mostly uses "[PATCH] subsystem: ...".  Occasionally I see
>> "[PATCH somegitrepo ...] ..." when it's necessary to explicitly say
>> whose git repo the patch needs to go through, but that's pretty rare.
>
> We do both. "foo: blah" is for subsystem "foo" of Git itself, but
> all-caps "JGIT PATCH" is "this is not even for Git". I don't know that
> the kernel really has an equivalent.
>
> -Peff

[net] and [net-next] are for the *tree* not the subsystem of the tree.

net: also means the networking subsystem, but it's different you might
have a new feature and say

[net-next PATCH v2] net: my awesome new feature.

Thanks,
Jake

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jeff King]

On Mon, Sep 19, 2016 at 04:55:50PM -0700, Josh Triplett wrote:

> On Mon, Sep 19, 2016 at 04:46:06PM -0700, Jacob Keller wrote:
> > On Mon, Sep 19, 2016 at 4:40 PM, Josh Triplett  
> > wrote:
> > > On Mon, Sep 19, 2016 at 04:34:35PM -0700, Jeff King wrote:
> > >> As far as your patch goes, I'd be OK with defining:
> > >>
> > >>   --rfc::
> > >>   Pretend as if `--subject-prefix='RFC PATCH'` was given.
> > >>
> > 
> > Would:
> > 
> > Shorthand for `--subject-prefix='RFC PATCH'`
> > 
> > be a better reading? I feel like using "pretend" is a bit weird here.
> 
> My patch used "Alias for"; if you prefer "Shorthand for" I'm
> indifferent. :)

Or maybe "Act as if...".

-Peff

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jeff King]

On Mon, Sep 19, 2016 at 04:40:22PM -0700, Josh Triplett wrote:

> >   - there are a non-trivial number of patches for other projects (JGIT,
> > EGIT, StGit, etc). This is somewhat unique to git, where we discuss
> > a lot of related projects on the list. But I wonder if other
> > projects would use subsystems in a similar way (though I guess for
> > the kernel, there are separate subsystems lists, so the "to" or "cc"
> > header becomes the more interesting tag).
> 
> The kernel mostly uses "[PATCH] subsystem: ...".  Occasionally I see
> "[PATCH somegitrepo ...] ..." when it's necessary to explicitly say
> whose git repo the patch needs to go through, but that's pretty rare.

We do both. "foo: blah" is for subsystem "foo" of Git itself, but
all-caps "JGIT PATCH" is "this is not even for Git". I don't know that
the kernel really has an equivalent.

-Peff

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jacob Keller]

On Mon, Sep 19, 2016 at 4:55 PM, Josh Triplett  wrote:
> On Mon, Sep 19, 2016 at 04:46:06PM -0700, Jacob Keller wrote:
>> On Mon, Sep 19, 2016 at 4:40 PM, Josh Triplett  wrote:
>> > On Mon, Sep 19, 2016 at 04:34:35PM -0700, Jeff King wrote:
>> >> As far as your patch goes, I'd be OK with defining:
>> >>
>> >>   --rfc::
>> >>   Pretend as if `--subject-prefix='RFC PATCH'` was given.
>> >>
>>
>> Would:
>>
>> Shorthand for `--subject-prefix='RFC PATCH'`
>>
>> be a better reading? I feel like using "pretend" is a bit weird here.
>
> My patch used "Alias for"; if you prefer "Shorthand for" I'm
> indifferent. :)

Alias seems fine to me.

Thanks,
Jake

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Josh Triplett]

On Mon, Sep 19, 2016 at 04:46:06PM -0700, Jacob Keller wrote:
> On Mon, Sep 19, 2016 at 4:40 PM, Josh Triplett  wrote:
> > On Mon, Sep 19, 2016 at 04:34:35PM -0700, Jeff King wrote:
> >> As far as your patch goes, I'd be OK with defining:
> >>
> >>   --rfc::
> >>   Pretend as if `--subject-prefix='RFC PATCH'` was given.
> >>
> 
> Would:
> 
> Shorthand for `--subject-prefix='RFC PATCH'`
> 
> be a better reading? I feel like using "pretend" is a bit weird here.

My patch used "Alias for"; if you prefer "Shorthand for" I'm
indifferent. :)

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jacob Keller]

On Mon, Sep 19, 2016 at 4:40 PM, Josh Triplett  wrote:
> On Mon, Sep 19, 2016 at 04:34:35PM -0700, Jeff King wrote:
>> As far as your patch goes, I'd be OK with defining:
>>
>>   --rfc::
>>   Pretend as if `--subject-prefix='RFC PATCH'` was given.
>>

Would:

Shorthand for `--subject-prefix='RFC PATCH'`

be a better reading? I feel like using "pretend" is a bit weird here.

That being said, I'm not super strongly against it, but just find that
it sounds weird for documentation.

Thanks,
Jake

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jacob Keller]

On Mon, Sep 19, 2016 at 4:34 PM, Jeff King  wrote:
>   - there are a non-trivial number of patches for other projects (JGIT,
> EGIT, StGit, etc). This is somewhat unique to git, where we discuss
> a lot of related projects on the list. But I wonder if other
> projects would use subsystems in a similar way (though I guess for
> the kernel, there are separate subsystems lists, so the "to" or "cc"
> header becomes the more interesting tag).

Working in the net-next community, we often use "[net PATCH]",
"[net-next] [PATCH]", or even just replace "PATCH" with "[net-next]"
and similar (though this is served just fine by --subject-prefix, and
may be an artifact caused because -P doesn't exist).

For the netdev, there are both "net" and "net-next" trees, and so
there is some need to distinguish between these. I prefer "[net
PATCH]" style myself.

I think --rfc is common enough to warrant its own tag, even if we add
"-P tag" just because it encourages its use for whenever you want to
comment about a patch without necessarily wanting it immediately
applied.

I also happen to prefer "RFC PATCH" instead of "PATCH/RFC" but I think
that's mostly preference.

>
> So I dunno what all this means. I do think "--rfc" to standardize on
> _some_ form of "RFC PATCH" or "PATCH/RFC" would serve the most people,
> and would make things more consistent. But at least in Git, people would
> be served by having arbitrary prefixes, too.
>

A general way to do this would be helpful, but i don't think it avoids
the usefulness of --rfc on its own.

I know that some formats are also generated by tools such as stgit
which has its own way to generate emails and doesn't use exactly the
same format as git.

Thanks,
Jake

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Josh Triplett]

On Mon, Sep 19, 2016 at 04:34:35PM -0700, Jeff King wrote:
> On Mon, Sep 19, 2016 at 01:44:08PM -0700, Josh Triplett wrote:
> 
> > On Mon, Sep 19, 2016 at 10:49:17AM -0700, Junio C Hamano wrote:
> > > Andrew Donnellan  writes:
> > > 
> > > > Sounds good to me. Agreed that "RFC" is essentially the only prefix
> > > > other than "PATCH" that I see, at least in the kernel.
> > > 
> > > Around here I think we saw WIP too, and that makes me lean towards
> > > Peff's earlier suggestion to allow an end-user supplied string in
> > > front of PATCH, i.e. "-P RFC" => "--subject-prefix='RFC PATCH'",
> > > even though I understand that those who _ONLY_ care about RFC would
> > > prefer --rfc (5 keystrokes) over "-P RFC" (6 keystrokes).
> > 
> > I do share the concern raised elsewhere in the thread that adding new
> > format-patch short options potentially conflicts with diff/rev-list
> > short options.  If you're not worried about that, I'd be happy to add
> > (and document and test) -P.  However, I'd still advocate adding --rfc as
> > well; it's a common case, and "-P RFC" is actually rather more
> > keystrokes when you count shifting. :)
> > 
> > There might also be some value in steering people towards "RFC" (since a
> > WIP is in a way an RFC).
> 
> Good point. This may be an opportunity to be just slightly opinionated
> and nudge people towards a micro-standard.
> 
> I was curious what we have used over the years on the git list. So here
> are the top hits for:
> 
>   # start with a maildir archive
>   find git/cur -type f |
> 
>   # grab first subject line from each mail
>   xargs grep -i -m1 -h ^subject: |
> 
>   # pull out only bracketed text, ignore "re: [PATCH]"
>   perl -lne '/subject:\s*(\[.*?\])/i and print $1' |
> 
>   # normalize numbers; note that a long patch series will be
>   # over-represented, since it gets one hit per message
>   perl -pe 's/[0-9]+/X/g' |
> 
>   # and then sort by count
>   sort | uniq -c | sort -rn
> 
> The top 5 hits are:
> 
>   26252 [PATCH X/X]
>   18255 [PATCH]
>   17262 [PATCH vX X/X]
>2330 [PATCH vX]
>2297 [PATCHvX X/X]
> 
> which is not surprising (our "-v" uses "PATCH vX", which is why that's
> so much more common). After that it starts to get interesting, but let's
> do two further transformations before the sort to coalesce similar
> cases:
> 
>   # drop versioning entirely
>   perl -pe 's/\s*vX//' |
> 
>   # drop multipart subjects
>   perl -pe 's{\s*X/X}{}' |
> 
> That gives us:
> 
>   67081 [PATCH]
>1286 [PATCH/RFC]
>1169 [RFC/PATCH]
> 863 [JGIT PATCH]
> 832 [ANNOUNCE]
> 797 [RFC]
> 675 [RFC PATCH]
> 537 [StGit PATCH]
> 524 [EGIT PATCH]
> 367 [BUG]
> 169 [StGIT PATCH]
> 158 [GUILT]
> 142 [PATCH RFC]
> 131 [WIP PATCH]
> 129 [PATCH/WIP]
> 115 [TopGit PATCH]
> 115 [PATCH VX]
> ...
> 
> Some of those are obviously uninteresting (like "ANNOUNCE" or "BUG").
> But I see a few interesting patterns:
> 
>   - the "slash" form of RFC/PATCH or PATCH/RFC seems more common than a
> straight prefix (2400 versus about 800)
> 
>   - both RFC/PATCH and PATCH/RFC seems similarly popular (but "RFC
> PATCH" is much more popular than "PATCH RFC")
> 
>   - WIP is a lot less popular; it seems reasonable that it's a synonym
> for RFC and I don't mind pushing people in that direction
> 
>   - there are a non-trivial number of patches for other projects (JGIT,
> EGIT, StGit, etc). This is somewhat unique to git, where we discuss
> a lot of related projects on the list. But I wonder if other
> projects would use subsystems in a similar way (though I guess for
> the kernel, there are separate subsystems lists, so the "to" or "cc"
> header becomes the more interesting tag).

The kernel mostly uses "[PATCH] subsystem: ...".  Occasionally I see
"[PATCH somegitrepo ...] ..." when it's necessary to explicitly say
whose git repo the patch needs to go through, but that's pretty rare.

> As far as your patch goes, I'd be OK with defining:
> 
>   --rfc::
>   Pretend as if `--subject-prefix='RFC PATCH'` was given.
> 
> for now. If we later add:
> 
>   -P ::
>   Pretend as if `--subject-prefix=' PATCH'` was given.
> 
> then `--rfc` naturally becomes:
> 
>   --rfc::
>   Pretend as if `-P RFC` was given.
> 
> in a backwards-compatible way. It doesn't have to all come at once, and
> it sounds like `-P` may not be as useful for the kernel (though I'd be
> interested if somebody wanted to do a similar count; I don't have a copy
> of the kernel archive handy).

Sounds reasonable to me.  I'll send v3 of --rfc with the requested
additional documentation fix, then.

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jeff King]

On Mon, Sep 19, 2016 at 01:44:08PM -0700, Josh Triplett wrote:

> On Mon, Sep 19, 2016 at 10:49:17AM -0700, Junio C Hamano wrote:
> > Andrew Donnellan  writes:
> > 
> > > Sounds good to me. Agreed that "RFC" is essentially the only prefix
> > > other than "PATCH" that I see, at least in the kernel.
> > 
> > Around here I think we saw WIP too, and that makes me lean towards
> > Peff's earlier suggestion to allow an end-user supplied string in
> > front of PATCH, i.e. "-P RFC" => "--subject-prefix='RFC PATCH'",
> > even though I understand that those who _ONLY_ care about RFC would
> > prefer --rfc (5 keystrokes) over "-P RFC" (6 keystrokes).
> 
> I do share the concern raised elsewhere in the thread that adding new
> format-patch short options potentially conflicts with diff/rev-list
> short options.  If you're not worried about that, I'd be happy to add
> (and document and test) -P.  However, I'd still advocate adding --rfc as
> well; it's a common case, and "-P RFC" is actually rather more
> keystrokes when you count shifting. :)
> 
> There might also be some value in steering people towards "RFC" (since a
> WIP is in a way an RFC).

Good point. This may be an opportunity to be just slightly opinionated
and nudge people towards a micro-standard.

I was curious what we have used over the years on the git list. So here
are the top hits for:

  # start with a maildir archive
  find git/cur -type f |

  # grab first subject line from each mail
  xargs grep -i -m1 -h ^subject: |

  # pull out only bracketed text, ignore "re: [PATCH]"
  perl -lne '/subject:\s*(\[.*?\])/i and print $1' |

  # normalize numbers; note that a long patch series will be
  # over-represented, since it gets one hit per message
  perl -pe 's/[0-9]+/X/g' |

  # and then sort by count
  sort | uniq -c | sort -rn

The top 5 hits are:

  26252 [PATCH X/X]
  18255 [PATCH]
  17262 [PATCH vX X/X]
   2330 [PATCH vX]
   2297 [PATCHvX X/X]

which is not surprising (our "-v" uses "PATCH vX", which is why that's
so much more common). After that it starts to get interesting, but let's
do two further transformations before the sort to coalesce similar
cases:

  # drop versioning entirely
  perl -pe 's/\s*vX//' |

  # drop multipart subjects
  perl -pe 's{\s*X/X}{}' |

That gives us:

  67081 [PATCH]
   1286 [PATCH/RFC]
   1169 [RFC/PATCH]
863 [JGIT PATCH]
832 [ANNOUNCE]
797 [RFC]
675 [RFC PATCH]
537 [StGit PATCH]
524 [EGIT PATCH]
367 [BUG]
169 [StGIT PATCH]
158 [GUILT]
142 [PATCH RFC]
131 [WIP PATCH]
129 [PATCH/WIP]
115 [TopGit PATCH]
115 [PATCH VX]
...

Some of those are obviously uninteresting (like "ANNOUNCE" or "BUG").
But I see a few interesting patterns:

  - the "slash" form of RFC/PATCH or PATCH/RFC seems more common than a
straight prefix (2400 versus about 800)

  - both RFC/PATCH and PATCH/RFC seems similarly popular (but "RFC
PATCH" is much more popular than "PATCH RFC")

  - WIP is a lot less popular; it seems reasonable that it's a synonym
for RFC and I don't mind pushing people in that direction

  - there are a non-trivial number of patches for other projects (JGIT,
EGIT, StGit, etc). This is somewhat unique to git, where we discuss
a lot of related projects on the list. But I wonder if other
projects would use subsystems in a similar way (though I guess for
the kernel, there are separate subsystems lists, so the "to" or "cc"
header becomes the more interesting tag).

So I dunno what all this means. I do think "--rfc" to standardize on
_some_ form of "RFC PATCH" or "PATCH/RFC" would serve the most people,
and would make things more consistent. But at least in Git, people would
be served by having arbitrary prefixes, too.

I don't have a big opinion on ordering or slashes. The slashes make
sense to me as "this is a patch, and it has these attribute tags" (so
we could even start saying "PATCH/maint" or something, I guess). And
"RFC" functions as such a tag. But for subsystems, putting the tag first
is probably best; I don't care about EGIT patches, so I can immediately
ignore them when it's at the beginning.

As far as your patch goes, I'd be OK with defining:

  --rfc::
Pretend as if `--subject-prefix='RFC PATCH'` was given.

for now. If we later add:

  -P ::
Pretend as if `--subject-prefix=' PATCH'` was given.

then `--rfc` naturally becomes:

  --rfc::
Pretend as if `-P RFC` was given.

in a backwards-compatible way. It doesn't have to all come at once, and
it sounds like `-P` may not be as useful for the kernel (though I'd be
interested if somebody wanted to do a similar count; I don't have a copy
of the kernel archive handy).

-Peff

PS There are some amusing ones as you get far down the list, like "DRY
   HUMOR PATCH". Clearly we need "-P" to encourage such things. :)

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Josh Triplett]

On Mon, Sep 19, 2016 at 10:49:17AM -0700, Junio C Hamano wrote:
> Andrew Donnellan  writes:
> 
> > Sounds good to me. Agreed that "RFC" is essentially the only prefix
> > other than "PATCH" that I see, at least in the kernel.
> 
> Around here I think we saw WIP too, and that makes me lean towards
> Peff's earlier suggestion to allow an end-user supplied string in
> front of PATCH, i.e. "-P RFC" => "--subject-prefix='RFC PATCH'",
> even though I understand that those who _ONLY_ care about RFC would
> prefer --rfc (5 keystrokes) over "-P RFC" (6 keystrokes).

I do share the concern raised elsewhere in the thread that adding new
format-patch short options potentially conflicts with diff/rev-list
short options.  If you're not worried about that, I'd be happy to add
(and document and test) -P.  However, I'd still advocate adding --rfc as
well; it's a common case, and "-P RFC" is actually rather more
keystrokes when you count shifting. :)

There might also be some value in steering people towards "RFC" (since a
WIP is in a way an RFC).

> >> +--rfc::
> >> +  Alias for `--subject-prefix="RFC PATCH"`. Use this when
> >> +  sending an experimental patch for discussion rather than
> >> +  application.
> >
> > Perhaps mention the phrase "Request For Comment" for the benefit of
> > those who aren't familiar ...
> 
> Good point.

I'll add that to the documentation in v3.

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Junio C Hamano]

Andrew Donnellan  writes:

> Sounds good to me. Agreed that "RFC" is essentially the only prefix
> other than "PATCH" that I see, at least in the kernel.

Around here I think we saw WIP too, and that makes me lean towards
Peff's earlier suggestion to allow an end-user supplied string in
front of PATCH, i.e. "-P RFC" => "--subject-prefix='RFC PATCH'",
even though I understand that those who _ONLY_ care about RFC would
prefer --rfc (5 keystrokes) over "-P RFC" (6 keystrokes).

>> +--rfc::
>> +Alias for `--subject-prefix="RFC PATCH"`. Use this when
>> +sending an experimental patch for discussion rather than
>> +application.
>
> Perhaps mention the phrase "Request For Comment" for the benefit of
> those who aren't familiar ...

Good point.

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Andrew Donnellan]

On 17/09/16 17:21, Josh Triplett wrote:

This provides a shorter and more convenient alias for
--subject-prefix='RFC PATCH'.

Includes documentation in the format-patch manpage, and a new test
covering --rfc.

Signed-off-by: Josh Triplett 


Sounds good to me. Agreed that "RFC" is essentially the only prefix 
other than "PATCH" that I see, at least in the kernel.


I don't have anything to say about the code, though I did note that 
there's a error message stating that "--subject-prefix and -k are 
mutually exclusive." - I haven't tested the patch, but I imagine this 
message will trigger with --rfc as well and could be slightly confusing.



---
v2:
- Add documentation to the format-patch manpage
- Call subject_prefix_callback rather than reimplementing it
- Update test to move expectations inside

 Documentation/git-format-patch.txt |  8 +++-
 builtin/log.c  |  8 
 t/t4014-format-patch.sh|  9 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 9624c84..b9590a5 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -19,7 +19,8 @@ SYNOPSIS
   [--start-number ] [--numbered-files]
   [--in-reply-to=Message-Id] [--suffix=.]
   [--ignore-if-in-upstream]
-  [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) ]
+  [--rfc] [--subject-prefix=Subject-Prefix]
+  [(--reroll-count|-v) ]
   [--to=] [--cc=]
   [--[no-]cover-letter] [--quiet] [--notes[=]]
   []
@@ -172,6 +173,11 @@ will want to ensure that threading is disabled for `git 
send-email`.
allows for useful naming of a patch series, and can be
combined with the `--numbered` option.

+--rfc::
+   Alias for `--subject-prefix="RFC PATCH"`. Use this when
+   sending an experimental patch for discussion rather than
+   application.


Perhaps mention the phrase "Request For Comment" for the benefit of 
those who aren't familiar (which admittedly, among users of 
git-format-patch, are probably rather few, but still).


--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH v2] format-patch: Add --rfc for the common case of [RFC PATCH]

[Jeff King]

On Sat, Sep 17, 2016 at 12:21:52AM -0700, Josh Triplett wrote:

> This provides a shorter and more convenient alias for
> --subject-prefix='RFC PATCH'.
> 
> Includes documentation in the format-patch manpage, and a new test
> covering --rfc.
> 
> Signed-off-by: Josh Triplett 
> ---
> v2:
> - Add documentation to the format-patch manpage
> - Call subject_prefix_callback rather than reimplementing it
> - Update test to move expectations inside

Assuming we want this option, the implementation looks good to me (and I
don't have a big opinion the first part of that sentence).

-Peff