On 2018-10-16 07:57, Junio C Hamano wrote:
> Rasmus Villemoes <rv@xxxxxxxxxxxxxxxxxx> writes:
>
>>> I wonder if it would make it easier to grok if we made the logic
>>> inside out, i.e.
>>>
>>> if ($sc eq $sender) {
>>> ...
>>> } else {
>>> if ($what =~ /^Signed-off-by$/i) {
>>> next if $suppress_cc{'sob'};
>>> } elsif ($what =~ /-by$/i) {
>>> next if $suppress_cc{'misc'};
>>> } elsif ($what =~ /^Cc$/i) {
>>> next if $suppress_cc{'bodycc'};> }
>>
>> ...yes, that's probably more readable.
>
> OK, unless there is more comments and suggestions for improvements,
> can you send in a final version sometime not in so distant future so
> that we won't forget?
Will do, I was just waiting for more comments to come in.
It may be surprising to existing users that
> the command now suddenly adds more addresses the user did not think
> would be added, but it would probably be easy enough for them to
> work around.
Yeah, I thought about that, but unfortunately the whole auto-cc business
is not built around some config options where we can add a new and
default false. Also note that there are also cases currently where the
user sends off a patch series and is surprised that lots of intended
recipients were not cc'ed (that's how I picked this subject up again; I
had a long series where I had put specific Cc's in each patch, at v2,
some of those had given a Reviewed-by, so I changed the tags, and a
--dry-run told me they wouldn't get the new version).
I suppose one could make use of -by addresses dependent on a new opt-in
config option, but that's not very elegant. Another option is, for a
release or two, to make a little (more) noise when picking up a -by
address - something like setting a flag in the ($what =~ /-by/) branch,
and testing that flag somewhere in send_message(). But I suppose the
message printed when needs_confirm eq "inform" is generic enough.
Rasmus
