Re: [PATCH v4 10/13] send-email: lazily load modules for a big speedup
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 27, 2021, 01:11 UTC
- Message-ID
- <xmqqv9759e0z.fsf@gitster.g>
- In-Reply-To
- <patch-10.13-61e3e3c93c5-20210524T074932Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 10 quoted lines
> -use File::Temp qw/ tempdir tempfile /;
> -use File::Spec::Functions qw(catdir catfile);
> ...
> + require File::Spec;
> + push @files, grep { -f $_ } map { File::Spec->catfile($f, $_) }
> sort readdir $dh;
> ...
> - push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);
> + require File::Temp;
> + push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);I suspect I would not be the only one who finds it curious to have the distinction between the use of "->" in File::Spec->catfile() and "::" in File::Temp::tempdir (and all the other function calls touched by this patch).
Wouldn't it be more faithful to the original to do:
require File::Spec::Functions;
push @files, ... map { File::Spec::Functions::catfile($f, $_) }in the first hunk? It would save the time readers of "git log -p" has to spend on wondering why "catfile" is an oddball.
Of course "man File::Spec" would educate readers where the difference comes from, but it is not immediately obvious to me why we want to switch in a "lazily load for speedup" that is supposed to be optimization-only, even if the class method ends up calling the same underlying thing.
Thanks.