From: Jonathan Nieder Date: Wed, 26 Apr 2017 00:07:41 GMT Subject: Re: [PATCH v2] sequencer: require trailing NL in footers Message-ID: <20170426000741.GS28740@aiede.svl.corp.google.com> In-Reply-To: <20170425235741.32546-1-jonathantanmy@google.com> Jonathan Tan wrote: > Reported-by: Brian Norris > Signed-off-by: Jonathan Tan > --- [...] > sequencer.c | 11 +++++++++++ > t/t3511-cherry-pick-x.sh | 14 ++++++++++++++ > 2 files changed, 25 insertions(+) Reviewed-by: Jonathan Nieder This is still pretty subtle (using the added newline that is added after a non-footer to turn the invalid footer into a valid one), but * it is clear from the code that it will work correctly * the new test ensures we'll continue to support this case * it is understandable after a little head scratching * I'm hoping the subtlety will go away once the code learns to deal with ordinary non-footer text that has a missing newline [...] > --- a/t/t3511-cherry-pick-x.sh > +++ b/t/t3511-cherry-pick-x.sh > @@ -208,6 +208,20 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo > test_cmp expect actual > ' > > +test_expect_success 'cherry-pick -x handles commits with no NL at end of message' ' > + pristine_detach initial && > + sha1=$(printf "title\n\nSigned-off-by: a" | git commit-tree -p initial mesg-with-footer^{tree}) && nit: Should this use a more typical sign-off line with an email address, to avoid a false-positive success in case git becomes more strict about its signoffs in the future? Something like printf "title\n\nSigned-off-by: S. I. Gner " >msg && sha1=$(git commit-tree -p initial mesg-with-footer^{tree}