On Wed, Nov 1, 2017 at 12:14 AM, Jeff King <peff@xxxxxxxx> 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
