All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Jeff King <peff@peff.net>
Cc: Philip Oakley <philipoakley@iee.org>,
	Junio C Hamano <gitster@pobox.com>,
	Christian Couder <christian.couder@gmail.com>,
	git <git@vger.kernel.org>, Ben Peart <Ben.Peart@microsoft.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	Mike Hommey <mh@glandium.org>,
	Lars Schneider <larsxschneider@gmail.com>,
	Eric Wong <e@80x24.org>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: Re: On the nature of cover letters [WAS Re: [PATCH 0/6] Create Git/Packet.pm]
Date: Wed, 1 Nov 2017 09:42:27 -0700	[thread overview]
Message-ID: <CAGZ79kYKK69Xw0-2OxFpo9Q=Kv1hvw8D7YkfhMFFcgzTuevTCQ@mail.gmail.com> (raw)
In-Reply-To: <20171101071422.c2k4plhntlgpdnbk@sigill.intra.peff.net>

On Wed, Nov 1, 2017 at 12:14 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 30, 2017 at 11:29:37AM -0700, Stefan Beller wrote:
>
>> > I can live with fancily-formatted cover letters. BUT. I would say if
>> > your cover letter is getting quite long, you might consider whether some
>> > of its content ought to be going elsewhere (either into commit messages
>> > themselves, or into a design document or other place inside the repo).
>>
>> I am of the opinion that in an ideal workflow the cover letter would be
>> part of the merge commit message. It may even be editorialized or
>> annotated by the maintainer performing the merge, but not necessarily so.
>>
>> Currently I rarely pay attention to merges, because they are not exciting
>> (in a good way). I mostly know the texts that Junio puts into the merge
>> message[1] from the cooking email, and otherwise there is not much
>> information added there.
>
> Yes, I'd agree that for some topics it would be nice for the merge
> message to give any "big picture" details that wouldn't have made sense
> inside a single commit message.
>
>> However looking at *what* Junio puts there, is "why is this worthwhile
>> to merge from the *projects* point of view?", whereas the cover letter
>> most of the time talks about "Why the *contributor* wants this merged".
>> Often these are subtly different, so it would be nice to have these
>> two competing views around.
>
> Yes, there's really no reason we couldn't have both (e.g., Junio's
> maintainer synopsis followed by a marker, and then the original author's
> cover letter).
>
> The workflow within git is a little awkward, though. You'd want to pick
> up the cover letter information via "git am" when the branch is applied.
> But it doesn't go into a commit message until the merge. So where is it
> stored in the meantime? You'd need a branch->msg key/value store of some
> kind.

branch.<name.>description already exists on the contributors side.
Maybe we can teach git-am to populate that field with either the
message-id only (of the coverletter), or the cover letter text.

Or we introduce a git-am hook, that could populate the value
of that setting.

Once we have that setting there, our man page of git-merge claims
merging pays attention to that setting via git-fmt-merge-msg.

I guess if we put these pieces in place, it may be easier to convince
Junio to try out that workflow.

> Junio's workflow now actually uses the "pu" merges as the storage
> location while a topic is working its way up. But there's a period
> between "am" and running the integration cycle where it would need to go
> somewhere else.

An empty commit on top is a clunky idea. (Though from the data model,
that empty commit "just learns about a new parent" in the merge process).
I think the branch description field will do.

> One other consideration is that the cover letter may get updated between
> rounds (e.g., because you changed patches in response to review, or even
> to discuss alternatives that came up and were rejected). That places a
> burden on the maintainer to update the prospective merge-message.

if there was a git-am hook, that could be smart enough to *always*
update the branch description to the latest cover letter, (or even just
the latest patch of the series, if just the last patch changed)

> So it may make more sense just to cross-reference those merges with the
> topics that spawned them on the mailing list. I.e., instead of copying
> the cover letter contents, just record the message-id (and update it
> whenever a new iteration of a topic is picked up via "git am"). That
> lets you get the cover letter information _and_ see any discussion
> or review around the patch.

That sounds good.

Stefan

  reply	other threads:[~2017-11-01 16:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-30 18:29 On the nature of cover letters [WAS Re: [PATCH 0/6] Create Git/Packet.pm] Stefan Beller
2017-11-01  7:14 ` Jeff King
2017-11-01 16:42   ` Stefan Beller [this message]
2017-11-01 22:31     ` Stefan Beller
2017-11-02  7:22       ` Jeff King
2017-11-02  7:48         ` Junio C Hamano
2017-11-02  8:01           ` Jeff King
2017-11-02 18:03             ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGZ79kYKK69Xw0-2OxFpo9Q=Kv1hvw8D7YkfhMFFcgzTuevTCQ@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=larsxschneider@gmail.com \
    --cc=mh@glandium.org \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.