Re: [RFC] cover-at-tip

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 10, 2017, 18:28 UTC
Message-ID: <20171110102825.ae7ccb37a13c5904d252faa1@google.com>
In-Reply-To: <e1d3ab5b-82e6-8490-8f2e-00c1359c6deb@suse.de>

On Fri, 10 Nov 2017 16:37:49 +0100 Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> wrote:

> > Hi,
> >
> > I'm starting to look into the cover-at-tip topic that I found in the leftover bits (http://www.spinics.net/lists/git/msg259573.html)

Thanks - I personally would find this very useful.

Show 5 quoted lines

> > Here's a first draft of a patch that adds support for format-patch --cover-at-tip. It compiles and works in my nice and user firnedly test case.
> > Just wanted to make sure I was going roughly in the right direction here.
> >
> >
> > I was wondering where is the right place to put a commit_is_cover_at_tip() as the test will be needed in other place as the feature is extended to git am/merge/pull.

I think you can put this in (root)/commit.c, especially since that test operates on a "struct commit *".

> > Feel free to comment. I know the help is not clear at this point and there's still some work to do on option handling (add a config option, probably have --cover-at-tip imply --cover-letter, etc) and
> > some testing :)

Both are good ideas. You should probably use a --cover-letter={no,auto,yes} instead of the current boolean, so that the config can use the same options and configuring it to "auto" (to use a cover letter if the tip is empty and singly-parented, and not to use a cover letter otherwise) is meaningful.

Show 6 quoted lines

> The proposed patch for format-patch does not output any "---" to signal the end of the commit log and the begining of the patch in the cover letter.
> This means that the log summary, the diffstat and the git footer ( --\n<git version>) is seen as part of the commit log. Which is just wrong.
> 
> Removing them would solve the issue but I feel they bring some useful info (or they would not be here).
> Adding a "---" between the commit log and those added infos poses another problem: git am does not see an empty patch anymore.
> I would need to add "some" level of parsing to am.c to make sure the patch content is just garbage and that there are no actual hunks for that.

Could you just take the message from the commit and put that in the cover letter? The summary and diffstat do normally have useful info, but if the commit is specifically made to be used only for the cover letter, I think that is no longer true.