Subject: Re: [PATCH 1/4] gitweb: notes feature

From: Jakub Narebski <[email-masked]>
Date: Fri, 5 Feb 2010 01:44:49 +0100

Jakub Narebski wrote:
> On Thu, 4 Feb 2010, Junio C Hamano wrote:
>> Junio C Hamano <[email-masked]> writes:
>> 
>>> Giuseppe Bilotta <[email-masked]> writes:
>>>
>>>> +		my %notes = () ;
>>>> +		foreach my $note_ref (@note_refs) {
>>>> +			my $obj = "$note_ref:$co{'id'}";
[...]
 
> Second, parse_commit / parse_commits use
> 
>   git rev-list -z --parents --header --max-count-X
> 
> If this command automatically shows notes (or it can be modified to
> automatically show notes) after unindented "Notes:" line (as per
> git-notes documentation), then the only thing that needs to be
> changed to fill %commit{'notes'} is parse_commit_text subroutine.

This command automatically shows notes, even in absence of GIT_NOTES_REF
environment variable or core.notesRef (if core.notesRef is not unset),
so unless we want gitweb to display notes flattened into commit message
(which I think was intended behavior - design decision for notes 
display), we would need to modify parse_commit_text to gather notes 
into %commin{'notes'} (or something).

> There would be no need for extra subroutine (and extra command, or
> even up to two extra commands per note) to get notes data.

You are right that we would need it if we want to display notes from
non-default namespace.

Still, 1 or 2 git commands per commit is a bit too much (with shortlog
displaying 100 commits per page): that is what "git cat-file --batch"
was invented ;-)

-- 
Jakub Narebski
Poland