Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder wrote: > Brandon Casey wrote: > >> --- /dev/null >> +++ b/t/t3511-cherry-pick-x.sh > [...] >> +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming >> footer' ' > > IIUC this is an illustration of false-positives from messages like > this one: > > base: do something great without a sign-off > > If he does that, it will be the best thing in the > world: or so I think. > > A worthy cause. Could the example broken message be tweaked to > emphasize that use case? With the current example, I'd consider > either result (blank line or no blank line) to be ok behavior by git. The primary motivation for this test was to exercise an existing behavior which fails to append a newline and sob if the last line of the last paragraph matches the sob of the committer regardless of whether the entire paragraph would be interpreted as a conforming footer. Your example is tested as a side-effect of that. I'll tweak the string so it looks like this: The signed-off-by string should begin with the words Signed-off-by followed by a colon and space, and then the signers name and email address. e.g. Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> > [...] >> +test_expect_success 'cherry-pick -s refrains from adding duplicate trailing >> sob' ' > > And the other side of basic "-s" functionality. > > One more test would be interesting: what does "-s" do when asked to > produce a duplicate signoff with an interspersed signoff by someone else? > > test: a patch with a more complicated life > > This patch bounced from $GIT_COMMITTER_NAME to Ms. Thor for > tweaking, then back to $GIT_COMMITTER_NAME who will be > recording it in permanent history. > > Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" > Signed-off-by: A U Thor This one exists as "adds sob when last sob doesn't match committer". In this case an additional sob should be appended to the footer. -Brandon -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
Brandon Casey wrote: >I'll tweak > the string so it looks like this: > > The signed-off-by string should begin with the words Signed-off-by followed > by a colon and space, and then the signers name and email address. e.g. > Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> Yep, that looks better than the example I suggested. :) [...] > On Tue, Jan 22, 2013 at 12:17 AM, Jonathan Nieder wrote: >> One more test would be interesting: what does "-s" do when asked to >> produce a duplicate signoff with an interspersed signoff by someone else? [...] > This one exists as "adds sob when last sob doesn't match committer". So it does. Thanks for the sanity check. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality
Brandon Casey wrote: > --- /dev/null > +++ b/t/t3511-cherry-pick-x.sh > @@ -0,0 +1,111 @@ [...] > + test_commit "$mesg_one_line" foo b mesg-one-line && > + git reset --hard initial && > + test_commit "$mesg_no_footer" foo b mesg-no-footer && > + git reset --hard initial && > + test_commit "$mesg_broken_footer" foo b mesg-broken-footer && > + git reset --hard initial && > + test_commit "$mesg_with_footer" foo b mesg-with-footer && > + git reset --hard initial && > + test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob && Neat. [...] > +test_expect_success 'cherry-pick -s inserts blank line after one line > subject' ' In particular, a blank line after a one-line subject that starts with the usual "subsystem:" convention is not mistaken for an RFC2822-style header. Good. [...] > +test_expect_failure 'cherry-pick -s inserts blank line after non-conforming > footer' ' IIUC this is an illustration of false-positives from messages like this one: base: do something great without a sign-off If he does that, it will be the best thing in the world: or so I think. A worthy cause. Could the example broken message be tweaked to emphasize that use case? With the current example, I'd consider either result (blank line or no blank line) to be ok behavior by git. [...] > +test_expect_success 'cherry-pick -s inserts blank line when conforming > footer not found' ' This is a simpler version of the previous test, without the distracting colon. [...] > +test_expect_success 'cherry-pick -s adds sob when last sob doesnt match > committer' ' Nice test for basic "-s" functionality. [...] > +test_expect_success 'cherry-pick -s refrains from adding duplicate trailing > sob' ' And the other side of basic "-s" functionality. One more test would be interesting: what does "-s" do when asked to produce a duplicate signoff with an interspersed signoff by someone else? test: a patch with a more complicated life This patch bounced from $GIT_COMMITTER_NAME to Ms. Thor for tweaking, then back to $GIT_COMMITTER_NAME who will be recording it in permanent history. Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" Signed-off-by: A U Thor With the changes suggested above or similar ones, Reviewed-by: Jonathan Nieder -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
