Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 30, 2008, 21:55 UTC
Message-ID: <200806302355.50560.jnareb@gmail.com>
In-Reply-To: <48682934.6090406@gmail.com>

On Mon, 30 Jun 2008, Lea Wiemann wrote:

Show 11 quoted lines

> Jakub Narebski wrote:
>>
>> some time ago I have send a patch which converted support for
>> links with hash and without action to use redirect instead of
>> silently filling correct action based on type of object [...]
>> IMHO it is better as it should prevent bookmarking "expensive" URL.
> 
> Haha, I'd actually suggest the opposite. ;-)  Figuring out the right
> action is almost free since you have to fetch the object anyways, so I
> doubt it'll make any difference performance-wise (though it'd be
> interesting to benchmark this).

It is one fork more[*1*], which doesn't matter on operating systems like Linux where forking is fairly cheap.

But perhaps avoiding redirect would be better solution... well, yet another solution would be to leave it up to gitweb configuration ;-)

[*1*] Or pre-fetching object (together with its type) which might be large, into memory.

Show 5 quoted lines

> However, gitweb's URLs are 
> prohibitively long -- so that nobody uses them in email --, and
> (automatically?) dropping the action parameter where possible would be a
> good first step to shortening them.  Another idea would be to shorten
> the hashes.

About shortening the hashes: _usually_ 8. characters is enough... but sometimes it isn't. What then?

[...]

>> [Git::Commit:] you should not error out on unknown header in commit object,
> 
> Unless this can actually happen in practice, I'd rather die aggressively
> -- it prevents errors (like cache hiccups) slipping through unnoticed.

If Git::Commit is to be *generic* object oriented interface to git repository (and not only for gitweb), it has either to skip unknown commit headers, or save them as-is.

As to checking cache: you can always check if header name is sane...

-- 
Jakub Narebski
Poland