On Fri, Aug 20, 2010 at 9:46 PM, Greg Stein <gstein_at_gmail.com> wrote:
> Getting back to this...
>
> On Thu, Aug 12, 2010 at 16:10, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> Recently, we've updated the various client APIs which do commits to
>> return commit info back through a callback[1], since they may be
>> extended to perform multiple commits (see issue #1199 for instance).
>> In doing so, I've had the opportunity to take a look at the
>> svn_commit_info_t struct. It's pretty simplistic, but includes fields
>> for the author and date.
>>
>> In 1.6 (I believe) we changed the various log and commit APIs to use a
>> hash of arbitrary revision properties, rather than a hard coded list.
>> I wonder if it's worth it to do so in the commit_info struct. We'd
>> still keep the existing fields for compat, but we would also add a
>> hash of revision properties, for consistency with the other APIs, and
>> for greater future extensibility.
>>
>> Thoughts?
>
> 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.
:/
>>
>> -Hyrum
>>
>> [1] The callback was added as a member of the client context, instead
>> of a per-API func/baton set. This choice was somewhat arbitrary, and
>> conversations with Mark regarding the same in the JavaHL wrappers have
>> made me wonder if we should go with the explicit func/baton args,
>> rather than using the client ctx. Anybody have any strong feelings
>> about this issue?
>
> 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.
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.
> 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. :(
-Hyrum
Received on 2010-08-20 23:44:04 CEST