Re: [PATCH 7/7] gitweb: group remote heads

Giuseppe Bilotta wrote:

On Mon, Sep 20, 2010 at 1:02 AM, Jakub Narebski [off-list ref] wrote:

quoted

When thinking about how display information about remotes and
remote-tracking branches in 'summary' view, we have to consider that
this view is meant to be what it says: a *summary*.  That is why both
the 'heads' and the 'tags' section displays only 15 items.

Without grouping remote heads the natural solution was to simply repeat
what we used for 'heads' subsection, namely limit displaying to 15
remote-tracking branches in 'remotes' subsection of the 'summary' view.

With grouping it is more complicated.  We can limit number of remote
head *per remote*, we can limit number of remotes... but what makes
IMVHO least sense is limit number of remote heads *then* do grouping.

This is something I absolutely agree with, BTW. It is the way it's
implemented currently because it was the quickest way to ship a
prototype out for discussion.

All right.  Prototyping is good.

quoted

The solution (1) i.e. limiting number of remote heads per remote, with
or without limiting number of remotes behaves, as you wrote, most
similarly to other components of 'summary' view.  On the other hand
with large number of remotes, and large number of remote heads in those
remotes it might be too large for a *summary* view.

So you maintain that limiting the amount of data in summary view
should be primary wrt to limiting the amount of time?

Well, what really affect gitweb performance is calling git commands, both
because of fork overhead, and because it means disk access (and gitweb
performance from what I have heard is affected mainly by IO, and not CPU).
With grouping (displaying remotes) the difference between displaying
remote-tracking branches (or information from them) and not displaying
them is an argument to git-for-each-ref.  So I don't think it would 
affect performance much.

quoted

The solution (3) i.e. displaying only list of remotes (perhaps limited
to 15 remotes) is simple and fast to render.  On the other hand it offers
least information and might be too little in the case of single remote.

If time spent processing is not an issue, we can retrieve the number
of heads for each remote and display that, for example. Or even play
with some more dynamic stuff like making each group collapsible,
starting with it collapsed and then display the content when the user
hovers it with the mouse, for example.

The dynamic stuff is IMHO a good idea... provided we can either do it
without JavaScript, or we can ensure that browser supports JavaScript
(see current hack used for turning 'blame' into 'blame_incremental'
view in gitweb).

Yet another solution would be to display only abbreviated list of remotes
if its more of them than some threshold, and list remotes with abbreviated
list of remote-tracking branches if there are only a few remotes.
 

quoted

quoted

If we go with option 3, it does make sense to get all remote names and
all remote branches at once, and thus to make the git_get_remotes()
call do all of the work.

Not if subroutine is called git_get_remotes(), IMVHO.  Perhaps
git_group_remotes()?

That I can do.

quoted

If remote doesn't have remote-tracking branches associated with it
(e.g. push-only remote, or remote predating separate remotes layout)
the value would be undef for given remote as key in hash.

Yes, this is something I have to take into consideration. Skip
displaying them is probably the best idea (unless we have other ways
to gather information about them).

Right.

P.S. It is not necessary for this series, but I think we should think
about "single remote" view... also because your code currently links
to such views, which do not exist yet (remotes/<remote> in path_info:
how it would be represented in CGI query format?).

-- 
Jakub Narebski
Poland