Re: [PATCHv2 GSOC 04/11] gitweb: Create Gitweb::Config module

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

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

Show 8 quoted lines

> Create Gitweb::Config module in 'gitweb/lib/Gitweb/Config.pm'
> to store all the configuration variables and subroutines
> regarding the gitweb.perl script.
> 
> This module depends only on $git_dir from Gitweb::Git and
> includes the configuration related variables and
> subroutines (Including those required for configuration
> of gitweb features).

Here it might be worth noting that subroutines related to per-repository config (feature_* subroutines) are not in this module, I guess because of (inter)dependencies.

Show 17 quoted lines

> 
> Subroutines moved:
> 	evaluate_gitweb_config
> 	configure_gitweb_features
> 	filter_snapshot_fmts
> 	gitweb_get_feature
> 	gitweb_check_feature
> 
> Update gitweb/Makefile to install Gitweb::Config module alongside gitweb
> 
> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
> ---
>  gitweb/Makefile             |    1 +
>  gitweb/gitweb.perl          |  495 ++----------------------------------------
>  gitweb/lib/Gitweb/Config.pm |  498 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 522 insertions(+), 472 deletions(-)
>  create mode 100644 gitweb/lib/Gitweb/Config.pm

[...]

Show 5 quoted lines

> -our $version = "++GIT_VERSION++";
> +# Only configuration variables with build-time overridable
> +# defaults are listed below. The complete set of variables
> +# with their descriptions is listed in Gitweb::Config.
> +$version = "++GIT_VERSION++";

Ah, I see that here you address (somewhat) the issue of moving
descriptions of build-time configuration variables out of gitweb.perl
into individual module(s).
  
[...]

Nice straight refactoring. It's a pity that git-diff cannot show this as code movement + some changes, like it would in the case of wholesame file rename.

-- 
Jakub Narebski
Poland