Re: [PATCH v2 02/11] gitweb: git_get_heads_list accepts an optional list of refs.

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Fri, Nov 14, 2008 at 7:48 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Dnia czwartek 13. listopada 2008 23:49, Giuseppe Bilotta napisał:
>
>> git_get_heads_list(limit, dir1, dir2, ...) can now be used to retrieve
>> refs/dir1, refs/dir2 etc. Defaults to ('heads') or ('heads', 'remotes')
>> depending on the remote_heads option.
>
> Minor nit: I think it would be better to use the same terminology in
> commit message as in code, i.e. 'class1' instead of 'dir1', or perhaps
> 'ref_class1' if it would be better.

Uhm, ref/ref_class1 reads horrible, but sticking with a uniform
terminology is a good point. I adjusted the commit message
consequently.

> This is only a suggestion, but perhaps this patch could be squashed
> with a later one?

Or with the previous one, since as you remark it's a generalization of
the previous.

>>       my @headslist;
>>
>> -     my ($remote_heads) = gitweb_check_feature('remote_heads');
>> -
>>       open my $fd, '-|', git_cmd(), 'for-each-ref',
>>               ($limit ? '--count='.($limit+1) : ()), '--sort=-committerdate',
>>               '--format=%(objectname) %(refname) %(subject)%00%(committer)',
>> -             'refs/heads', ( $remote_heads ? 'refs/remotes' : '')
>> +             @refs
>>               or return;
>>       while (my $line = <$fd>) {
>>               my %ref_item;
>
> So this is a bit of generalization of (part of) previous patch,
> isn't it?

Precisely. I must say I had problems finding the proper splitting
point for some of these patches, because they had a very organic
evolution, but at the same time sqashing them together would give too
large changesets at once. You'll find that this is not the only patch
that makes the most sense only after seeing what comes later.

-- 
Giuseppe "Oblomov" Bilotta