On Wed, Apr 04, 2012 at 04:31:42PM +0200, Jakub Narebski wrote:
> On Wed, 4 April 2012, Kacper Kornet wrote:
> > On Wed, Apr 04, 2012 at 01:12:01AM +0200, Jakub Narebski wrote:
> >> On Tue, 3 Apr 2012, Kacper Kornet wrote:
> > What about:
> > $no_list_age::
> > Whether to show the column with date of the most current commit on the
> > projects list page. It can save a bit of I/O.
> Perhaps it would be better to say it like this:
> $no_list_age::
> If true, omit the column with date of the most current commit on the
> projects list page. [...]
> It is true that it can save a bit of I/O: the git_get_last_activity()
> examines all branches (some of which are usually loose), and must hit
> the object database, unpacking/getting commit objects to get at commit
> date.
> But the fact that it also saves a fork (a git command call) per repository
> reminds me of something which I missed in first round of review, namely
> that generating 'age' and 'age_string' fields serve also as a check if
> repository really exist.
> So either we document this fact, or use some other way to verify that
> git repository is valid.
I think that git_project_list_body always works with the list returned
by git_get_projects_list. And git_get_projects_list validates if the
path is a git repository. So it should not be a problem. Please correct
me, if I am wrong.
> >>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> >>> index a8b5fad..f42468c 100755
> >>> --- a/gitweb/gitweb.perl
> >>> +++ b/gitweb/gitweb.perl
> >>> @@ -133,6 +133,9 @@ our $default_projects_order = "project";
> >>> # (only effective if this variable evaluates to true)
> >>> our $export_ok = "++GITWEB_EXPORT_OK++";
> >>> +# don't generate age column
> >>> +our $no_list_age = 0;
> >> "age" column where?
> >> Hmmm... can't we come with a better name than $no_list_age?
> > Any of $no_age_column, $omit_age_column, $no_last_commit would be better?
> $no_age_column seems better than $no_list_age... but see below.
> >>> @@ -5495,7 +5500,8 @@ sub git_project_list_body {
> >>> 'tagfilter' => $tagfilter)
> >>> if ($tagfilter || $search_regexp);
> >>> # fill the rest
> >>> - @projects = fill_project_list_info(\@projects);
> >>> + my @all_fields = $no_list_age ? ('descr', 'descr_long', 'owner', 'ctags', 'category') : ();
> >>> + @projects = fill_project_list_info(\@projects, @all_fields);
> >> That looks quite strange on first glance. I know that empty list means
> >> filling all fields, but the casual reader migh wonder about this
> >> conditional expression.
> >> Perhaps it would be better to write it this way:
> >> - @projects = fill_project_list_info(\@projects);
> >> + my @fields = qw(descr descr_long owner ctags category);
> >> + push @fields, 'age' unless ($no_list_age);
> >> + @projects = fill_project_list_info(\@projects, @fields);
> >> or something like that.
> >> Well, at least until we come up with a better way to specify "all fields
> >> except those specified".
> > Yes, that's better. Especially that I would like also to introduce
> > option to prevent printing repository owner everywhere.
> Well, because this patch affects gitweb configuration, and because we
> need to preserve (as far as possible) the backward compatibility with
> existing gitweb configuration files we need to be careful with changes.
> Perhaps instead of $no_age_column that can be single configuration
> variable like @excluded_project_list_fields instead of one variable
> per column. Somebody might want to omit project description as well
> (though then project search must be limited to project names only).
> Though this approach will have problem that some of columns simply
> have to be present... maybe one variable per column (perhaps hidden
> in a hash) is a better solution.
I thought about two different variables as that would have a slightly
different functionality. While I want to get rid off Last Change from
the projects list page, I still want to get this information on pages of
single repositories. On the other hand I don't want repository owner to
be shown anywhere.
--
Kacper Kornet
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html