Samuel GROOT <[email protected]> writes:

> @@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' '
>  test_expect_success $PREREQ 'setup expect' "
>  cat >expected-suppress-body <<\EOF
>  0001-Second.patch
> -(mbox) Adding cc: A <[email protected]> from line 'From: A 
> <[email protected]>'
> -(mbox) Adding cc: One <[email protected]> from line 'Cc: One 
> <[email protected]>, [email protected]'
> -(mbox) Adding cc: [email protected] from line 'Cc: One <[email protected]>, 
> [email protected]'
> -(cc-cmd) Adding cc: [email protected] from: './cccmd'
> +Adding cc: A <[email protected]> from From: header
> +Adding cc: One <[email protected]> from Cc: header
> +Adding cc: [email protected] from Cc: header
> +Adding cc: [email protected] from: './cccmd'

This hunk differs from the others a bit. I totally agree that removing
the (mbox) prefix makes sense, but you're removing (cc-cmd) here, which
did carry some information.

I'd write it as

Adding cc: [email protected] from --cc-cmd: ./cccmd

It might make sense to split this into two patches: one for (mbox) +
headers and one for (cc-cmd) and (to-cmd). Spotting special-cases like
the above inside a long patch is hard for reviewers.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to