From: Jakub Narębski <jnareb@gmail.com>
Date: Wed, 27 Nov 2013 15:56:08 GMT
Subject: Re: [PATCH] gitweb: Add an option for adding more branch refs
Message-ID: <52961618.1070603@gmail.com>
In-Reply-To: <1385462243-7898-1-git-send-email-krzesimir@endocode.com>

Krzesimir Nowak wrote:

> Overriding an @additional_branch_refs configuration variable with
> value ('wip') will make gitweb to show branches that appear in
> refs/heads and refs/wip (refs/heads is hardcoded). Might be useful for
> gerrit setups where user branches are not stored under refs/heads/.
>

The description of this change starts with technical details,
instead of starting with intent of this change.

Perhaps (this is only a proposal)

   Introduce @additional_branch_refs configuration variable, holding
   names of references to be considered branches; by default empty.
   For example setting it to ('wip') will make gitweb ...


BTW. I have thought at first that is something similar to 'remote_heads'
feature, which among others adds 'remotes' section to 'summary' view
displaying refs/remotes/* refs... but no, gitweb still doesn't treat 
refs/remotes as branches, even with this feature set.

Nb. why new configuration variable, and not new %feature?

> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
> ---
>  gitweb/gitweb.perl | 99 ++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 74 insertions(+), 25 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 68c77f6..9bfd38b 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -17,6 +17,7 @@ use Encode;
>  use Fcntl ':mode';
>  use File::Find qw();
>  use File::Basename qw(basename);
> +use List::Util qw(min);
>  use Time::HiRes qw(gettimeofday tv_interval);
>  binmode STDOUT, ':utf8';
>
[...]
 > @@ -3184,24 +3210,43 @@ sub git_get_project_owner {
 >   	return $owner;
 >   }
 >
 > -sub git_get_last_activity {
 > -	my ($path) = @_;
 > -	my $fd;
 > +sub git_get_last_activity_age {
 > +	my ($refs) = @_;
 > +	my $fd = -1;
 >
 > -	$git_dir = "$projectroot/$path";
 >   	open($fd, "-|", git_cmd(), 'for-each-ref',
 >   	     '--format=%(committer)',
 >   	     '--sort=-committerdate',
 >   	     '--count=1',
 > -	     'refs/heads') or return;
 > +	     $refs) or return undef;

git-for-each-ref accepts more than one pattern. Why not simply

    	open($fd, "-|", git_cmd(), 'for-each-ref',
    	     '--format=%(committer)',
    	     '--sort=-committerdate',
    	     '--count=1',
   -	     'refs/heads') or return;
   +	     get_branch_refs()) or return;

Then we won't need List::Util::min.

[...]
 > +sub git_get_last_activity {
 > +	my ($path) = @_;
 > +	my @ages = ();
 > +
 > +	$git_dir = "$projectroot/$path";
 > +	for my $ref (get_branch_refs()) {
 > +		my $age = git_get_last_activity_age('refs/' . $_);
 > +
 > +		push @ages, $age if defined $age;
 > +	}
 > +	if (@ages) {
 > +		my $min_age = min(@ages);
 > +
 > +		return ($min_age, age_string($min_age));
 > +	}
 > +
 >   	return (undef, undef);
 >   }
 >
[...]
-- 
Jakub Narębski