Re: [PATCHv2 GSOC 03/11] gitweb: Create Gitweb::Git module

From: Jakub Narebski <jnareb@gmail.com>
Date: Jul 15, 2010, 20:13 UTC
Message-ID: <201007152213.27998.jnareb@gmail.com>
In-Reply-To: <1279178951-23712-4-git-send-email-pavan.sss1991@gmail.com>

On Thu, 15 Jul 2010, Pavan Kumar Sunkara wrote:

Show 19 quoted lines

> Create a Gitweb::Git module in  'gitweb/lib/Gitweb/Git.pm'
> to deal with running git commands (and also processing output
> of git commands with external programs) from gitweb.
> 
> This module is intended as standalone module, which does not require
> (include) other gitweb' modules to avoid circular dependencies.  That
> is why it includes $GIT variable, even though this variable is
> configured during building gitweb.  On the other hand $GIT is more
> about git configuration, than gitweb configuration.
> 
> Subroutines moved:
> 	evaluate_git_version
> 	git_cmd
> 	quote_command
> 
> Update gitweb/Makefile to install Gitweb::Git module alongside gitweb
> 
> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
> ---

This change (this refactoring) is fairly uncontroversial...

> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl

[...]

Show 5 quoted lines

> -# core git executable to use
> -# this can just be "git" if your webserver has a sensible PATH
> -our $GIT = "++GIT_BINDIR++/git";
> +# $GIT is from Gitweb::Git
> +$GIT = "++GIT_BINDIR++/git";

[...]

> +# core git executable to use
> +# this can just be "git" if your webserver has a sensible PATH
> +our $GIT;

...but for addressing the discussion about whether to put (or rather leave) description of $GIT variable in main gitweb/gitweb.perl script, to have description of all build-time configuration variables in one place. You should at least say something about this issue either in commit message, or in the comments for this patch, in my opinion.

-- 
Jakub Narebski
Poland