On Wed, 17 Aug 2005, Madan U Sreenivasan wrote:
> Am working on svn issue #443, and am facing a problem. I thought I
> could start a discussion here before I go ahead and make the changes.
>
> A brief history of #443
> Issue at:
> http://subversion.tigris.org/issues/show_bug.cgi?id=443
> Some discussions at:
> http://svn.haxx.se/dev/archive-2005-07/0267.shtml
> Latest partial patches at:
> http://svn.haxx.se/dev/archive-2005-08/0115.shtml
> http://svn.haxx.se/dev/archive-2005-08/0408.shtml
>
> Working on step 3, I feel that this step should cover introducing a
> new parameter(post_commit_err) in the svn_commit_callback_t declaration.
> since svn_commit_callback_t is a public declaration, this would involve
> introducing new versions of the svn_commit_callback_t typedef, of the
> various functions that implement this type and the API functions that
> use these type/implementations.
>
> Quite a huge change. I would like comments on whether any alternative
> ideas (other than introducing a new parameter) are possible.
>
Nothing particular comes to mind. You could return an error, to
communicate this info, but then the other commit information gets lost.
But is it that a huge change? You need to rev svn_repos_get_commit_editor
and svn_ra_get_commit_editor. Are there that many callers of these?
> More importantly, the same set of changes are needed every time we
> need to change the svn_commit_callback_t prototype. am not comfortable
> with this. shouldnt it be easy to change the callback to accomodate any
> new info we might need to pass on to the client in future? Any comments?
>
>
This is a good point. In some other places, (notification and status),
we use a struct as a parameter instead of a collection of parameters. I
think this is the way to go for callbacks because structs can be extended.
So, why not do the same here?
I propose moving svn_client_commit_info2_t to svn_types.h and calling it
svn_commit_info_t (you don't need to rev any APIs since that's new). Then
rev svn_commit_callback_t to take a const svn_commit_info_t * (and a
baton, and why not a scratch pool while we are here (see issue #1881)). I
know that this will mean reworking some of your previous patches, but I
think that's the cleanest solution.
We all love API revving, don't we?
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 18 10:57:45 2005