git format-patch should honor notes

git format-patch should honor notes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

I'm just starting to experiment with 'git notes', because it seems to
fit well with my workflow on several projects, except for one drawback.

My workflow is that I post patch series for upstream review via 'git
send-email'.  Often, that results in feedback that requires me to
amend/rebase my series, and post a v2 or v3 of the series.  By adding
'git config notes.rewriteRef refs/notes/commits', I can add notes that
will carry across my rebase, and remind me what I changed in v2 (for
example, git notes add -m 'v2: fix foo, per mail xyz@example.com').
This is handy for me, and I think it is also handy for reviewers -
someone who took the time to read through v1 should know what I changed
in response to their comments, and only have to focus in on commits with
changes, rather than on the entire resent series.

However, I think such review helps are informational only - that is, in
'git send-email' parlance, they belong between the '-- ' and diffstat
lines of the email, and not in the upstream commit.  After all, once my
series is finally accepted upstream, it will no longer be rebased, and
'git bisect' sees only the final version.  I see no reason for the
commit message to carry the cruft of extra information that was only
helpful during reviewing the amended series, nor any reason why upstream
should carry around my notes.

So, what I'm missing is the ability for 'git send-email' (or more
fundamentally, 'git format-patch') to be able to include contents of a
particular (set of) notes reference in each patch file it generates,
where the note falls in the informative portion of the email, and is
intentionally omitted from the upstream commit when someone else runs
'git am' on my email.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: git format-patch should honor notes

[Junio C Hamano]

Eric Blake <eblake@redhat.com> writes:

> So, what I'm missing is the ability for 'git send-email' (or more
> fundamentally, 'git format-patch') to be able to include contents of a
> particular (set of) notes reference in each patch file it generates,
> where the note falls in the informative portion of the email, and is
> intentionally omitted from the upstream commit when someone else runs
> 'git am' on my email.

I do not know if "should" is the right word, but it certainly sounds like
it would be nice to have such an option for the usecase you described.

Re: git format-patch should honor notes

[Jeff King]

On Tue, Dec 07, 2010 at 02:53:09PM -0700, Eric Blake wrote:

> My workflow is that I post patch series for upstream review via 'git
> send-email'.  Often, that results in feedback that requires me to
> amend/rebase my series, and post a v2 or v3 of the series.  By adding
> 'git config notes.rewriteRef refs/notes/commits', I can add notes that
> will carry across my rebase, and remind me what I changed in v2 (for
> example, git notes add -m 'v2: fix foo, per mail xyz@example.com').
> This is handy for me, and I think it is also handy for reviewers -
> someone who took the time to read through v1 should know what I changed
> in response to their comments, and only have to focus in on commits with
> changes, rather than on the entire resent series.

Yeah, that is a workflow that some others have mentioned using here,
too. And I think there is general agreement that notes should go after
the "---" in format-patch. We just need a working patch.

Thomas posted one in February:

  http://article.gmane.org/gmane.comp.version-control.git/140819

But there were some issues and it never got polished. Michael suggested
that he does something similar here:

  http://article.gmane.org/gmane.comp.version-control.git/140819

but there was no indication on whether it happens manually or if he has
a patch. I don't know if anything else has happened in that area. I'm
sure if you feel like working on a patch it would be well received.

-Peff

Re: git format-patch should honor notes

[Michael J Gruber]

Jeff King venit, vidit, dixit 07.12.2010 23:11:
> On Tue, Dec 07, 2010 at 02:53:09PM -0700, Eric Blake wrote:
> 
>> My workflow is that I post patch series for upstream review via 'git
>> send-email'.  Often, that results in feedback that requires me to
>> amend/rebase my series, and post a v2 or v3 of the series.  By adding
>> 'git config notes.rewriteRef refs/notes/commits', I can add notes that
>> will carry across my rebase, and remind me what I changed in v2 (for
>> example, git notes add -m 'v2: fix foo, per mail xyz@example.com').
>> This is handy for me, and I think it is also handy for reviewers -
>> someone who took the time to read through v1 should know what I changed
>> in response to their comments, and only have to focus in on commits with
>> changes, rather than on the entire resent series.
> 
> Yeah, that is a workflow that some others have mentioned using here,
> too. And I think there is general agreement that notes should go after
> the "---" in format-patch. We just need a working patch.
> 
> Thomas posted one in February:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/140819
> 
> But there were some issues and it never got polished. Michael suggested
> that he does something similar here:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/140819
> 
> but there was no indication on whether it happens manually or if he has
> a patch. I don't know if anything else has happened in that area. I'm
> sure if you feel like working on a patch it would be well received.
> 
> -Peff

I do it with ":r!git notes show" in vim (after "/---"), which has the
advantage over "format-patch --show-notes" that the notes are not
indented nor preceded by a "Notes:" header. (I wouldn't mind the
latter.) This is comfortable enough to have kept me from writing a patch.

Also, in order to be really useful, I would need a place to store the
cover letter also. I was experimenting a while back with a design for
annotating branchnames which "basically" worked but haven't had time to
really implement it. If I remember correctly, I had to set up some
"bogus" refs to keep my notes from being garbage collected and was still
figuring out the best place to put them. I'll dig it up when I have time to.

Michael

Re: git format-patch should honor notes

[Johan Herland]

On Wednesday 08 December 2010, Michael J Gruber wrote:
> Also, in order to be really useful, I would need a place to store the
> cover letter also. I was experimenting a while back with a design for
> annotating branchnames which "basically" worked but haven't had time
> to really implement it. If I remember correctly, I had to set up some
> "bogus" refs to keep my notes from being garbage collected and was
> still figuring out the best place to put them. I'll dig it up when I
> have time to.

I believe the last time the issue of adding notes to branch names was 
discussed, the consensus was that rather than using notes, they could 
be stored using a custom entry in the config file, e.g.

  git config branch.mybranch.description "Description of mybranch"

I might have misremembered this, though.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git format-patch should honor notes

[Michael J Gruber]

Johan Herland venit, vidit, dixit 08.12.2010 11:12:
> On Wednesday 08 December 2010, Michael J Gruber wrote:
>> Also, in order to be really useful, I would need a place to store the
>> cover letter also. I was experimenting a while back with a design for
>> annotating branchnames which "basically" worked but haven't had time
>> to really implement it. If I remember correctly, I had to set up some
>> "bogus" refs to keep my notes from being garbage collected and was
>> still figuring out the best place to put them. I'll dig it up when I
>> have time to.
> 
> I believe the last time the issue of adding notes to branch names was 
> discussed, the consensus was that rather than using notes, they could 
> be stored using a custom entry in the config file, e.g.
> 
>   git config branch.mybranch.description "Description of mybranch"
> 
> I might have misremembered this, though.

They certainly "could". The question whether they "should" depends on
what they are used for:

- config is neither versioned nor easily shareable; perfect for your own
scratch notes to go away once work is done

- notes are versioned and can be shared (I don't need to tell you...);
perfect for longer term annotations you want to keep

Note that "sharing" here includes also pushing to your backup repo and
cloning around. I'd certainly put patch series cover letters in the
second category.

Michael

Re: git format-patch should honor notes

[Johan Herland]

On Wednesday 08 December 2010, Michael J Gruber wrote:
> Johan Herland venit, vidit, dixit 08.12.2010 11:12:
> > On Wednesday 08 December 2010, Michael J Gruber wrote:
> >> Also, in order to be really useful, I would need a place to store
> >> the cover letter also. I was experimenting a while back with a
> >> design for annotating branchnames which "basically" worked but
> >> haven't had time to really implement it. If I remember correctly,
> >> I had to set up some "bogus" refs to keep my notes from being
> >> garbage collected and was still figuring out the best place to put
> >> them. I'll dig it up when I have time to.
> >
> > I believe the last time the issue of adding notes to branch names
> > was discussed, the consensus was that rather than using notes, they
> > could be stored using a custom entry in the config file, e.g.
> >
> >   git config branch.mybranch.description "Description of mybranch"
> >
> > I might have misremembered this, though.
>
> They certainly "could". The question whether they "should" depends on
> what they are used for:
>
> - config is neither versioned nor easily shareable; perfect for your
> own scratch notes to go away once work is done
>
> - notes are versioned and can be shared (I don't need to tell
> you...); perfect for longer term annotations you want to keep
>
> Note that "sharing" here includes also pushing to your backup repo
> and cloning around. I'd certainly put patch series cover letters in
> the second category.

True. I was wrong to equate cover letters with local-only branch name 
descriptions.

As has been discussed before, you can use notes to store the cover 
letter, the question is which SHA-1 to attach it to.

Using a SHA-1 that doesn't exist in the repo (e.g. the SHA-1 of the 
branch name) leaves the note vulnerable to 'git notes prune', but maybe 
that is an acceptable restriction ('git notes prune' must be manually 
invoked in any case). For extra safety, we could add a config option 
that refuses 'git notes prune' for a given notes ref, something like:

  git config notes.mynotes.refusePrune true


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: git format-patch should honor notes

[Thomas Rast]

Jeff King wrote:
> > My workflow is that I post patch series for upstream review via 'git
> > send-email'.  Often, that results in feedback that requires me to
> > amend/rebase my series, and post a v2 or v3 of the series.  By adding
> > 'git config notes.rewriteRef refs/notes/commits', I can add notes that
> > will carry across my rebase, and remind me what I changed in v2 (for
> > example, git notes add -m 'v2: fix foo, per mail xyz@example.com').
> 
> Yeah, that is a workflow that some others have mentioned using here,

Incidentally it's what I wrote the rewriteRef support for :-)

> too. And I think there is general agreement that notes should go after
> the "---" in format-patch. We just need a working patch.
> 
> Thomas posted one in February:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/140819
> 
> But there were some issues and it never got polished.

I got pretty frustrated with gfp being rather brittle.  It is very
hard to insert anything anywhere in the output stream in such a way
that the output is not affected in any *other* scenario where this
option is disabled.

So I think a good angle of attack if you want to hack around on this
would be to clean up gfp so that it becomes easier to work on, and/or
come up with a better/cleaner place to insert the notes support than I
had.

That being said, the version I still use just shifts around a linefeed
after the ---, IIRC, and so far nobody complained about that in
practice ;-)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch