Re: [RFC-PATCH 2/2] t9001: adding --quote-mail option test
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- May 23, 2016, 20:05 UTC
- Message-ID
- <vpqshx8a6ak.fsf@anie.imag.fr>
- In-Reply-To
- <1464031829-6107-3-git-send-email-tom.russello@grenoble-inp.org>
> Subject: [RFC-PATCH 2/2] t9001: adding --quote-mail option test
We write messages at imperative tone, hence s/adding/add/
Tom Russello <tom.russello@grenoble-inp.org> writes:
> From: Tom Russello <tom.russello@ensimag.grenoble-inp.fr>
Please use the same identity for email and commit to avoid this line.
> --- > > > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
No diffstat again?
Splitting a patch series as "code first; tests after" is not a good idea IMHO. When questionning the behavior of To: Vs Cc: in the previous patch, I would have appreciated having tests in the same message, to check that the tested behavior was indeed the one I was reading in the code.
OTOH, having one patch to introduce "--quote-email populates To: and Cc: headers", and then another one for "--quote-email quotes the message body" would make the review much easier.
Oh, BTW, this obviously lacks documentation (Documentation/git-send-email.txt).
And that's one reason why the diffstat is useful: one can reply "this lacks tests and doc" before even reviewing the patch.
Regards,
-- Matthieu Moy http://www-verimag.imag.fr/~moy/