Re: [PATCH v5 1/2] send-email: refactor header generation functions
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 17, 2023, 15:13 UTC
- Message-ID
- <xmqqv8l59ot2.fsf@gitster.g>
- In-Reply-To
- <230117.86wn5lxpl0.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 19 quoted lines
>> +sub send_message {
>> + my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
>
> This makes the diff smaller, but if we're refactoring these functions to
> return arguments it's probably better to return a hash reference rather
> than remembering all the parameter names.
>
> But in this case it's probably fine...
> ...
>> +sub pre_process_file {
>> + my ($t, $quiet) = @_;
>
> This, I think, is an anti-pattern in this file. We can just read the
> "$quiet" and other things that we set up when we parse the parameters as
> globals, we don't need to pass them as named parameters.
>
> It doesn't help readability to shadow that variable with another lexical
> here below:
> ...Thanks as usual for a careful review, with concrete suggestions for improvements.