Re: [PATCH] gitweb: Support caching projects list
Hi,
On Fri, Mar 14, 2008 at 05:14:51AM -0700, Jakub Narebski wrote:
Show 21 quoted lines
> Petr Baudis <pasky@suse.cz> writes:
> [...]
> > + if ($cache_lifetime and -f $cache_file
> > + and stat($cache_file)->mtime + $cache_lifetime * 60 > time()
> > + and open (my $fd, $cache_file)) {
> > + $stale = time() - stat($cache_file)->mtime;
> > + my @dump = <$fd>;
> > + close $fd;
> > + # Hack zone start
> > + my $VAR1;
> > + eval join("\n", @dump);
> > + @projects = @$VAR1;
> > + # Hack zone end
>
> Why do you read line by line, only to join it, i.e.
> my @dump = <$fd>; ... join("\n", @dump);
> instead of slurping all file in one go:
> local $/ = undef; my $dump = <$fd>; ... $dump;
>
> Besides, why do you use Data::Dumper instead of Storable? Both are
> distributed with Perl; well, at least both are in perl-5.8.6-24. no particular reason - I simply never heard about Storable. I learned
Perl too long ago it seems. ;-)
Show 6 quoted lines
> [...]
> > - git_project_list_body(\@list, $order);
> > + git_project_list_body(\@list, $order, undef, undef, undef, undef, $projlist_cache_lifetime);
>
> This is ugly. Why not use hash for "named parameters", as it is done
> in a few separate places in gitweb (search for '%opts')?
I agree - I was simply too lazy to make another patch. :-)
--
Petr "Pasky" Baudis
Whatever you can do, or dream you can, begin it.
Boldness has genius, power, and magic in it. -- J. W. von Goethe
Petr Baudis <pasky@suse.cz>