Re: [PATCH v6] mailinfo: allow e-mail files as input
flat view
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:04
Nicolas Sebrecht [off-list ref] writes:
We traditionally allowed a mbox file or a directory name of a maildir to be ... Signed-off-by: Nicolas Sebrecht <redacted>
Thanks. I have one more comment on the test script, but it's something I can locally fix (iow, there is no need to resend your patch if there is no other issue pointed out by others, and if you agree to my suggested improvements).
quoted hunk
diff --git a/t/t4150-am.sh b/t/t4150-am.sh index a12bf84..4c99240 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh@@ -63,6 +63,53 @@ with the data reset to initial values. EOF +cat >rfc2822_email <<EOF +Return-Path: <user@domain.name> +X-Flags: 0000 + 999
Please quote that EOF if you do not mean to have any $variable substituted
in the "here document", like this:
cat >rfc2822_email <<\EOF
... here document comes here ...
EOF
Otherwise reviewers need to waste time looking for what is being replaced
in the huge here document to understand what is going on.
+Delivered-To: delivery to user@domain.name +Received: (qmail invoked by alias); 16 Jul 2009 05:25:49 -0000 +Received: from vger.knl.xyz (EHLO vger.knl.xyz) [4.3.2.1] + by mx0.gmx.com (mx-us004) with SMTP; 16 Jul 2009 01:25:49 -0400 ... + +EOF +
The headers look a bit too excessive to my taste, but probably you wanted to take a real-life example. If that is the case, I suspect the manually added X-Flags: at the beginning defeats that purpose, though. I'd suggest either removing the hand-munging, or triming the Received: sequence to make it a bit shorter.
quoted hunk
@@ -222,6 +269,13 @@ test_expect_success 'am takes patches from a Pine mailbox' ' test -z "$(git diff master^..HEAD)" ' +test_expect_success 'am takes patches from a RFC2822 formated email' ' + git checkout first && + cat rfc2822_email patch1 | git am && + ! test -d .git/rebase-apply && + test -z "$(git diff master^..HEAD)" +'
These days we tend to write the last step git diff --exit-code master^ HEAD which allows "sh t4150-am.sh -i -v" to be more useful when debugging. But the existing tests in this script are old fashioned, and you matched their style in this patch, which is very good. We probably would want a separate patch to modernize them after 1.6.4, though.