Subject: Re: [PATCHv2 5/6] gitweb: last-modified time should be commiter, not author

From: Jakub Narebski <[email-masked]>
Date: Thu, 5 Feb 2009 00:38:57 +0100

On Mon, 26 Jan 2009, Giuseppe Bilotta wrote:

> The last-modified time header added by RSS to increase cache hits from
> readers should be set to the date the repository was last modified. The
> author time in this respect is not a good guess because the last commit
> might come from a oldish patch.
> 
> Use the committer time for the last-modified header to ensure a more
> correct guess of the last time the repository was modified.

First, changing %latest_date from author time to committer time affects
not only Last-Modified HTTP header, but (after this series) also
various "publication dates" in the feed contents.  But I think that for
all those committer time is better approximation of publication date
and last change date than author time.

Second, author time reflects when change (commit) was made, according
to authors (perhaps skewed) clock.  Committer time reflects when given
commit (version of a commit) was entered into repository, or to be more
exact into some clone of given project.  But there is also an issue of
when changes got into given instance of repository (given clone): that
I guess might be found by stat-ing HEAD (if it arrived by commit),
FETCH_HEAD (if it arrived by fetch or pull) and ??? (if it arrived by
push)... err... it looks like it wouldn't work in most common case,
sorry, unless we want to stat all refs and packed-refs file. But while
this date might be better for Last-Modified, I'm not sure if it is good
at all for publication date.

So committer time is better than author time, and looks like good
middle ground.

> 
> Signed-off-by: Giuseppe Bilotta <[email-masked]>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 756868a..8c49c75 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6015,7 +6015,7 @@ sub git_feed {
>  	}
>  	if (defined($commitlist[0])) {
>  		%latest_commit = %{$commitlist[0]};
> -		%latest_date   = parse_date($latest_commit{'author_epoch'});
> +		%latest_date   = parse_date($latest_commit{'committer_epoch'});

Nice and simple. Good.

>  		print $cgi->header(
>  			-type => $content_type,
>  			-charset => 'utf-8',
> -- 
> 1.5.6.5

-- 
Jakub Narebski
Poland