Re: [PATCH] remove unused parameter from libsvn_client/copy.c:setup_copy()

On Sat, Jul 26, 2008 at 11:49:00PM +0100, Julian Foad wrote:
> > For starters, here's a patch that removes the force parameter
> > from setup_copy. OK to commit?
>
> Not sure.
>
> Strictly speaking, we should preserve the behaviour options for the
> existing APIs. The previous behaviours were well documented and thus
> something that third-party tools may well have been relying on. While we
> could argue that we have just "improved" the behaviour, I am not sure
> that we always pay enough attention to meeting our
> backward-compatibility guarantees.

You're right, the behaviour of the affected functions was
retroactively altered.

But does "behaviour-compatible" equal "binary-compatible"?

> To be on the safe side I would say we have introduced a regression in
> svn_client_copy4/3/2/1() and should fix it. This would probably involve
> a three-way option: options "fail" and "delete" for the old APIs, and
> option "move" for the new svn_client_copy5() API.
 
OK, so you want setup_copy() to be smart enough to tell from which
API rev it was called, and behave as advertised by that API revision,
instead of always behaving as advertised by the latest API revision.

Fine by me, but I'd like to hear other peoples' opinions on this, too.

> Can you see if it would be much work to do that?

I don't know. I have not looked at the code in detail, I was just
poking around because of a different problem and noticed that force
was unused. I don't have time to look into this in detail right now.

So, to summarise, we're seeing two issues here:

 * The API docs for svn_client_copy5 are flat out wrong with respect
   to the force parameter.

 * The behaviour of svn_client_copy4/3/2/1 has been retroactively
   changed.

Right?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-27 12:20:03 CEST