Re: [PATCH v2 02/11] gitweb: git_get_heads_list accepts an optional list of refs.
- From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
- Date
- Nov 14, 2008, 21:52 UTC
- Message-ID
- <cb7bb73a0811141352p6e46196cq9272b60bba89b951@mail.gmail.com>
- In-Reply-To
- <200811141948.57785.jnareb@gmail.com>
On Fri, Nov 14, 2008 at 7:48 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Show 9 quoted lines
> 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.
Show 15 quoted lines
>> 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