From: Giuseppe Bilotta Date: Fri, 14 Nov 2008 21:52:45 GMT Subject: Re: [PATCH v2 02/11] gitweb: git_get_heads_list accepts an optional list of refs. Message-ID: In-Reply-To: <200811141948.57785.jnareb@gmail.com> On Fri, Nov 14, 2008 at 7:48 PM, Jakub Narebski 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