Petr Baudis wrote: > [$repo->_cmd_output:] > > we _need_ such a wrapper _publically_, because it tends to be > actually the main use-case of Git.pm, Well, sure, I happen to not be convinced, but it *may* be useful. The point I'm trying to make is that it's not part of what I'm writing here. > as part of your gitweb migration to Git::Repo, you will temporarily > introduce calls to _cmd_output(), the "internal" API. :-) Sure, it's > only temporary, but many won't have the luxury to adjust the Git::Repo > API to provide all the operations they need, and ultimately they will > need to defer to the pipe interface. Yup, and I'm actually fine with that. (I'll probably alias _cmd_output to cmd_output in gitweb, just to make it clear that it is, for the purpose of gitweb, a *supported* mode of operation.) If the Git::Repo::_cmd_output API goes away, you'll have to insert a few lines of code in gitweb, but that's it. Really, no big deal. 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. > 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>.) >> 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. ;-) >> 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. -- Lea -- 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
