Re: [PATCH GSoC 1/3] gitweb: Create Gitweb::Config module

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 3, 2010, 15:54 UTC
Message-ID: <AANLkTikUmFA658jzd27cu1NmjJsV8T9Hkrd7z2WNY3R7@mail.gmail.com>
In-Reply-To: <20100603152030.GD20775@machine.or.cz>

On Thu, Jun 3, 2010 at 15:20, Petr Baudis <pasky@suse.cz> wrote:

> On Thu, Jun 03, 2010 at 07:25:54PM +0530, Pavan Kumar Sunkara wrote:

Show 6 quoted lines

>> -our $t0;
>> -if (eval { require Time::HiRes; 1; }) {
>> -     $t0 = [Time::HiRes::gettimeofday()];
>
> Why is this moved to Gitweb::Config? Shouldn't this be rather part of
> Gitweb::Request?

>> +our @ISA = qw(Exporter);

This is also re-arranging deck chairs on the Titanic, but 'use base qw(Exporter)' is nicer.

Show 9 quoted lines

>> +# __DIR__ is taken from Dir::Self __DIR__ fragment
>> +sub __DIR__ () {
>> +     File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
>>  }
>> -our $number_of_git_cmds = 0;
>> +use lib __DIR__ . "/lib";
>
> Wouldn't it be more elegant to use FindBin? I'm just not sure how long
> is it part of core Perl.

No, those don't do the same thing as discussed in previous reviews. FindBin finds the invoked binary, Dir::Self finds the the current file.