Re: [PATCH v2 03/11] gitweb: separate heads and remotes list in summary view

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Nov 14, 2008, 22:01 UTC
Message-ID: <cb7bb73a0811141401r748b5a4em4e5711935dd16a1@mail.gmail.com>
In-Reply-To: <200811142104.35019.jnareb@gmail.com>

2008/11/14 Jakub Narebski <jnareb@gmail.com>:

> Dnia czwartek 13. listopada 2008 23:49, Giuseppe Bilotta napisał:
>
> Very nice patch. Now that I have read it, I don't think it should be
> squashed with previous patch (well, again that is only a suggestion).

See reply in previous patch. Sometimes it's hard to tell what's the best patch-splitting strategy ...

> Barring one issue (see below) its conciseness shows that gitweb has
> quite good internal API.

Most definitely. I find that working on gitweb is almost pleasurable ;-) [I mean, it's still Perl, which is not Tcl but not Ruby either 8-P]

Show 20 quoted lines

>>       my @forklist;
>>       my ($check_forks) = gitweb_check_feature('forks');
>>
>> @@ -4535,6 +4537,13 @@ sub git_summary {
>>                              $cgi->a({-href => href(action=>"heads")}, "..."));
>>       }
>>
>> +     if (@remotelist) {
>> +             git_print_header_div('remotes');
>> +             git_heads_body(\@remotelist, $head, 0, 15,
>> +                            $#remotelist <= 15 ? undef :
>> +                            $cgi->a({-href => href(action=>"heads")}, "..."));
>> +     }
>> +
>
> The only problem is that link leads to list of _all_ heads (best case),
> or list to local branches (worst case, but I don't think gitweb does
> it), instead of only list of remotes refs (remote-tracking branches),
> as one would think.  Perhaps we could use 'h' (hash), or 'opt (extra
> options) parameter for this action, or just add 'remotes' action?

Adding a 'remotes' section (and corresponding action, too) is what is done by subsequent patches. It's quite obvious, I think, that the patch sequence follows _very_ closely the order in which I implemented/tested features. It might make more sense to move some of them earlier, but then such earlier patches would only make sense because of the ones that follow ... this is why I decided to keep the sequence as is.

>> 1.5.6.5
>
> P.S. Not uptodate (git version 1.6.0.4)? Just kidding...

Yeah, I know, given that I work with 'next' gitweb, I could as well just upgrade the system git to the same version 8-P

Instead, I'm just relying on stock Debian stuff, which obviously, being in freeze now, is not updating unstable either 8-P

-- 
Giuseppe "Oblomov" Bilotta