On Fri, Aug 20, 2010 at 17:43, Hyrum K. Wright
<hyrum_wright_at_mail.utexas.edu> wrote:
> On Fri, Aug 20, 2010 at 9:46 PM, Greg Stein <gstein_at_gmail.com> wrote:
>...
>> Sounds like something good for 1.8.
>
> I've actually got some of the bits already done, and figured since the
> commit callback stuff has already been monkeyed with in 1.7, we might
> as well go whole hog. I'm going to be trapped in an aluminium tube
> for several hours tomorrow, and may polish it off then.
>
> The questions that I've been pondering though are thus: do we fetch
> the revprops in a second request if the (presumably older) server
> doesn't send them down? Do we return the revprops as they look after
> the post-commit hook or before? Do we even allow modifying the
> revprops in a post-commit or other -commit hook? I may know enough
> about the problem to be dangerous, but not enough to be constructive.
> :/
I'm with Stefan on keeping 1.7 tight (have been, but he wrote a great
email making it very lucid). So for this, "Great idea. For 1.8." ...
go ahead and answer those questions yourself. I'll keep my brainpower
applied to wc-ng instead :-)
>...
>> Adding more crap to the client ctx is bad. Yeah, I have strong
>> feelings about it :-P ... I'd very much request adding it as
>> parameters to *only* the functions which require it.
>>
>> Once I get more Round Tuits, then I'd want to rev that context
>> structure to svn_client_ctx2 and strip it *way* down. Clean out
>> obsolete members, move some items into a private structure linked from
>> the publicly-visible context, and leave just a few that the client
>> should be able to set/get.
>
> Well, I think there is some value in constructing a set of default
> parameters for use by client functions. *Most* of our client APIs use
> the same wc notifier and the same wc notification baton, for instance.
Right. Like I said: there are *some* that make a bit of sense. The
notifier and cancellation are two. The wc_ctx is another.
But does svn_client_cat() require commit callbacks? Surely not. Nor
does it need a log message receiver. These kinds of things belong as
parameters.
> Having to construct and pass those around for several APIs is both
> tedious and error-prone, so a generic "give me some defaults and I'll
> customize it" does make some sense.
>
> It's the hodgepodge of APIs and parameters that we currently have
> that's a problem, some (like the wc notifier) are part of the context,
> while others aren't. It's the worst of both worlds.
Yup. And adding the commit callbacks makes it even worse. They are
clearly not common to the client API, but to only a few functions.
>> I have a similar problem with the merge_cmd_baton in client/merge.c.
>> It is a garbage can full of random things that people didn't want to
>> pass as arguments.
>
> Yeah, if we've got to have something like that internally, it says
> something about the complexity of the code. :(
I've looking at minimizing the params, and it is pretty hard. But I'd
rather have lots of parameters than a catch-all structure. Reasoning
about a function that has access to "everything" is damned hard. It is
effectively like having global variables in the code -- "which
variables are used, and where and how?" It is nearly impossible to
answer those kinds of questions easily when you lump crap into a
structure.
Cheers,
-g
Received on 2010-08-21 00:32:06 CEST