Re: [PATCH] send-email: Add tocmd option to suppress-cc
- From
Viresh Kumar <viresh.kumar@linaro.org>
- Date
- Jun 13, 2017, 03:43 UTC
- Message-ID
- <20170613034337.GF5297@vireshk-i7>
- In-Reply-To
- <xmqqmv9dnkut.fsf@gitster.mtv.corp.google.com>
On 12-06-17, 11:04, Junio C Hamano wrote:
Show 22 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: > > > Viresh Kumar <viresh.kumar@linaro.org> writes: > > > >> On 18-05-17, 17:05, Viresh Kumar wrote: > >>> This adds tocmd option to suppress-cc command which already supports > >>> cccmd and others. > >>> > >>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > >>> --- > >>> Documentation/git-send-email.txt | 1 + > >>> git-send-email.perl | 8 ++++---- > >>> 2 files changed, 5 insertions(+), 4 deletions(-) > >> > >> Ping !! > > > > Pong? > > > > I didn't get the impression that in the discussion that led to your > > patch (or in any response to the patch) we established that it is a > > good idea to change the behaviour of suppress-cc unconditionally > > like this patch does.
Yeah, there were no conclusions there but we agreed that we have a problem to solve. And this patch tried one of the ways.
Show 14 quoted lines
> I think suppress-cc that suppresses cccmd output is very sensible
> (after all, it is about suppressing CC and cccmd is about producing
> CC), but I am not convinced that it is a good idea to suppress tocmd
> (which is about producing To) via suppress-cc.
>
> Going back to the core part of your change, i.e.
>
> - foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
> + foreach my $entry (qw (tocmd cccmd cc author self sob body bodycc)) {
>
> to think about it a bit more, I notice that all the existing ones
> are about CC. So I am not just not convinced that tocmd belongs to
> the same class. I am inclined to say that it smells quite different
> from others.That's right but how do we solve this problem then?
Add another optional argument like suppress-to ? I thought that it would be better to override suppress-cc rather than attempting any such thing.
I am fine with any solution that address the concerns raised by this patch though.
-- viresh