Hi,
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:
Some discussions at:
Latest partial patches at:
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.
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?
Regards,
Madan.
To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

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:

Some discussions at:

Latest partial patches at:

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 (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Thu, 2005-08-18 at 14:26, Peter N. Lundblad wrote:
Wed, 17 Aug 2005, Madan U Sreenivasan wrote:
[snip]
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
hmmm, replace all usage of svn_client_commit_info2_t to
svn_commit_info_t too? ah, there goes my step 2 down the drain. :(
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
good idea overall. Thx.
I dont know the scratch pool funda. Do you mean that the callback should
take a pool as a parameter?
know that this will mean reworking some of your previous patches, but I
think that's the cleanest solution.
sadly, yes, but its okay if we can get some long term leverage because
of this.

We all love API revving, don't we?
really? ;)

Regards,
Maddy.

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Fri, 19 Aug 2005, Madan U Sreenivasan wrote:

Thu, 2005-08-18 at 14:26, Peter N. Lundblad wrote:
Wed, 17 Aug 2005, Madan U Sreenivasan wrote:
[snip]
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
good idea overall. Thx.
I dont know the scratch pool funda. Do you mean that the callback should
take a pool as a parameter?

Yes, for temporary allocations only. Mostly for consistency in this case.

know that this will mean reworking some of your previous patches, but I
think that's the cleanest solution.
sadly, yes, but its okay if we can get some long term leverage because
of this.

We all love API revving, don't we?
really? ;)

Nope. :-)

Regards,
//Peter

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Fri, 2005-08-19 at 16:37, Peter N. Lundblad wrote:
Fri, 19 Aug 2005, Madan U Sreenivasan wrote:

Thu, 2005-08-18 at 14:26, Peter N. Lundblad wrote:
Wed, 17 Aug 2005, Madan U Sreenivasan wrote:
[snip]
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
good idea overall. Thx.
I dont know the scratch pool funda. Do you mean that the callback should
take a pool as a parameter?

Yes, for temporary allocations only. Mostly for consistency in this case.
more question do I retain for
the libsvn_client, or use svn_commit_info_t?

If I have to use svn_commit_info_t everywhere, where does the
constructor go?

Thanks and Regards,
Madan.

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Fri, 19 Aug 2005, Madan U Sreenivasan wrote:

Fri, 2005-08-19 at 16:37, Peter N. Lundblad wrote:
Fri, 19 Aug 2005, Madan U Sreenivasan wrote:

Thu, 2005-08-18 at 14:26, Peter N. Lundblad wrote:
Wed, 17 Aug 2005, Madan U Sreenivasan wrote:
[snip]
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
good idea overall. Thx.
I dont know the scratch pool funda. Do you mean that the callback should
take a pool as a parameter?

Yes, for temporary allocations only. Mostly for consistency in this case.
more question do I retain for
the libsvn_client, or use svn_commit_info_t?

Just drop svn_client_commit_info2_t.

If I have to use svn_commit_info_t everywhere, where does the
constructor go?

I think it belongs in svn_types.h as well. We already have functions there
and a constructor "belongs" to the type in some sense.

Regards,
//Peter

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

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.

Excuse my bluntness, and this is not only directed at you, but I find this very
annoying. You won't get any input from me if you don't say what the subject of
issue #443 is, preferably in your email's "Subject:" line. I don't have time
to open every email on this list and follow the links in it or look it up in
the issue tracker just to find out whether I am interested in it.

I suspect other people would appreciate the same courtesy.

If you only wanted responses from people who are already familiar with it and
know the issue number by heart, then that's fine for you and them, but still
annoys other people who _might_ be interested if only they knew what it was about.
- Julian

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Fri, 2005-08-19 at 19:22, Julian Foad wrote:
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.

Excuse my bluntness, and this is not only directed at you, but I find this very
annoying. You won't get any input from me if you don't say what the subject of
issue #443 is, preferably in your email's "Subject:" line. I don't have time
to open every email on this list and follow the links in it or look it up in
the issue tracker just to find out whether I am interested in it.
oh, sorry. Sure thing. Would keep this in mind now on.

I suspect other people would appreciate the same courtesy.

If you only wanted responses from people who are already familiar with it and
know the issue number by heart, then that's fine for you and them, but still
annoys other people who _might_ be interested if only they knew what it was about.
was not intentionalsorry, again.

Regards,
Madan.

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org