Re: [PATCH v4 10/13] send-email: lazily load modules for a big speedup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason  <avarab@xxxxxxxxx> writes:

> -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.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux