Re: [PATCH 2/3] add new Git::Repo API

On Fri, Jul 18, 2008 at 08:09:48PM +0200, Lea Wiemann wrote:

Also, gitweb isn't using cmd_output because it needs a pipe interface,
but because it needs a caching layer in between -- most applications
would do just fine with open calls.

One of the points of the API is to abstract these out.

quoted

As I said, majority of Git API usage is actually the pipe API. So we
should figure out how to provide it. I agree that it's not immediately
within your scope, but you are introducing new Perl API and this just
needs to be embedded somewhere there consistently.

Sure, but pleeeease not as part of this patch series! :-)  Look, our
conversation is going something like this:

Lea: Here's a Perl API that fell out of my gitweb development for free.
Petr: I want a pony with the API!
Lea: But I don't have a pony.  Can we please just go with the Perl API
as a start, even if I don't supply ponies with it?

(Cf. the very cute <http://c2.com/cgi/wiki?IwantaPony>.)

I'm fine with that, as long as the version that enters into master will
have a pony so that we stay with a single pony within the codebase in
the end, not two ponies with differently shaped saddles.

But as I said, I'm going to work on that.

quoted

quoted

If you're getting a SHA1 through the user-interface, check its existence
with get_sha1 before passing it to the constructor.

But that's an expensive operation, you need extra Git exec for this,

For the gazillionth time in this thread, there is no extra exec.  It's a
write to a bidirectional cat-file --batch-check pipe.  It's not
expensive.  Really. ;-)

But the API is still obnoxiously elaborate, as I complained in another
mail.

quoted

quoted

I have resolving code in gitweb's git_get_sha1_or_die

The thing that concerns me about this is that this might show that your
approach to error handling is not flexible enough for some real-world
usage and this might be a design mistake - is that not so?

I don't think so; the error handling is fine.  Given that I want
fine-granular error reporting for gitweb, there *needs* to be a
git_get_sha1_or_die function; you can't move that into the API.

Wait, this doesn't compute here. The error handling is fine, but it is
actually not fine for gitweb. Can't we make it fine for everyone?

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie