You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "D.J. Heap" <dj...@shadyvale.net> on 2004/03/11 06:07:32 UTC
[PATCH] Re: The new svn_wc_adm_*_depth functions.
The latest iteration...
Log:
Change calls from the old svn_wc_adm_* functions to the new
svn_wc_adm_*_depth functions and mark the old functions as deprecated.
* subversion/include/svn_wc.h
(svn_wc_adm_open, svn_wc_adm_probe_open, svn_wc_adm_probe_try)
Mark as deprecated.
* subversion/libsvn_wc/wc.h
Tweak comment for svn_wc__adm_pre_open to refer to new function.
* subversion/libsvn_wc/copy.c
* subversion/libsvn_wc/log.c
* subversion/libsvn_wc/adm_ops.c
* subversion/libsvn_wc/status.c
* subversion/libsvn_wc/adm_files.c
* subversion/libsvn_wc/update_editor.c
* subversion/libsvn_wc/lock.c
* subversion/libsvn_client/switch.c
* subversion/libsvn_client/delete.c
* subversion/libsvn_client/externals.c
* subversion/libsvn_client/export.c
* subversion/libsvn_client/revisions.c
* subversion/libsvn_client/status.c
* subversion/libsvn_client/prop_commands.c
* subversion/libsvn_client/checkout.c
* subversion/libsvn_client/revert.c
* subversion/libsvn_client/diff.c
* subversion/libsvn_client/copy.c
* subversion/libsvn_client/log.c
* subversion/libsvn_client/update.c
* subversion/libsvn_client/resolved.c
* subversion/libsvn_client/commit_util.c
* subversion/libsvn_client/add.c
* subversion/libsvn_client/commit.c
* subversion/libsvn_client/relocate.c
* subversion/libsvn_client/url.c
* subversion/clients/cmdline/switch-cmd.c
* subversion/clients/cmdline/main.c
* subversion/clients/cmdline/propedit-cmd.c
* subversion/clients/cmdline/info-cmd.c
Call the new depth aware svn_wc_adm_* functions.
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Benjamin Pflugmann <be...@pflugmann.de>.
On Fri 2004-03-12 at 10:35:18 -0600, kfogel@collab.net wrote:
> Benjamin Pflugmann <be...@pflugmann.de> writes:
> > When reading source code which you are unfamiliar with, functions
> > named dothis(), dothis2(), dothis3() are one of the worst things,
> > IMHO.
>
> I think this "2" (or "N") suffix is actually something of a
> convention. I've seen it before in more than one place, and never
> been confused about what it meant: an API needed variants. Consider
> popen(), popen2(), popen3().
Bad example, I don't know Python :)
Well, I looked it up (that's why I know it's propably Python) and
popen2() and popen3() seem like an exception to me, because there is
some easy "memory hook" indeed (the number is the amount of pipes
returned), not simply an meaningless enumeration. Well, at least for
those two... popen4() seems be one of those abominations that I fear,
and I found no popen() in these Python docs so I don't comment on
that.
> It's hard to say in absolute terms "It's confusing" or "It's not
> confusing". All I can tell you is that I've not been confused by it,
> and other APIs have chosen a similar course.
If you have no problems with enumerations, and it's only me, fine.
When there's no problem, don't fix it. :-)
> I can put a note in HACKING about what '@deprecated' means -- i.e,
> that it's the *API* that's deprecated, not necessarily the name. We
> should simply assume an arbitrary amount of API change when 2.0 comes
> out. It's not like it'll be that hard for anyone to fix their apps.
> You try to compile your code, it doesn't work, you go do the obvious
> thing (removing the "N" suffixes from functions, basically).
Jupp, that wasn't my concern.
HAND,
Benjamin.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Benjamin Pflugmann <be...@pflugmann.de> writes:
> When reading source code which you are unfamiliar with, functions
> named dothis(), dothis2(), dothis3() are one of the worst things,
> IMHO.
I think this "2" (or "N") suffix is actually something of a
convention. I've seen it before in more than one place, and never
been confused about what it meant: an API needed variants. Consider
popen(), popen2(), popen3().
It's hard to say in absolute terms "It's confusing" or "It's not
confusing". All I can tell you is that I've not been confused by it,
and other APIs have chosen a similar course.
> There is no indication of what the "2" means except that is some kind
> of variation of the theme of dothis(). What then happens is that
> everytime one sees dothis() or dothis2(), one has to get back to the
> doc/definition to see which of them did what, because there is usually
> no easy way to build a "memory hook" out of an enumeration.
>
> I'd prefer any of
> - foo_pool()
> - foo_recommended()
> - foo_for_api_3_0()
>
> ...or whatever makes more sense to you. But please don't simply
> enumerate them. Pretty please?
I can put a note in HACKING about what '@deprecated' means -- i.e,
that it's the *API* that's deprecated, not necessarily the name. We
should simply assume an arbitrary amount of API change when 2.0 comes
out. It's not like it'll be that hard for anyone to fix their apps.
You try to compile your code, it doesn't work, you go do the obvious
thing (removing the "N" suffixes from functions, basically).
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Benjamin Pflugmann <be...@pflugmann.de>.
On Thu 2004-03-11 at 22:42:51 -0700, D.J. Heap wrote:
> kfogel@collab.net wrote:
> [snip]
> >
> >If we're going to fall back to the old names, then the "2" variants
> >might make the most sense.
> >
> >IMHO, we should do this. By resetting our APIs back to the
> >names-of-first-choice, we avoid names growing arbitrarily long (think
> >3.0).
> >
> >Thoughts?
>
> It looks like an agreement is forming on the '2' variants (it sounds
> reasonable to me) -- if there are no objections then I will wait another
> day or so and update the patch to rename the functions with that scheme
> and submit it for review?
May I humbly ask that you reconsider giving the function get an at
least somewhat meaningful name?
When reading source code which you are unfamiliar with, functions
named dothis(), dothis2(), dothis3() are one of the worst things,
IMHO.
There is no indication of what the "2" means except that is some kind
of variation of the theme of dothis(). What then happens is that
everytime one sees dothis() or dothis2(), one has to get back to the
doc/definition to see which of them did what, because there is usually
no easy way to build a "memory hook" out of an enumeration.
I'd prefer any of
- foo_pool()
- foo_recommended()
- foo_for_api_3_0()
...or whatever makes more sense to you. But please don't simply
enumerate them. Pretty please?
Bye,
Benjamin.
PS: If these go into 1.1.0 or the like, which is my current
understanding, we have to live with the name until 2.0, correct?
If not, ignore my plea. But if so, what if we need another change
after 1.1.0 is out with dothis2()? We get a dothis3() in 1.3.0 and
a dothis4() in 1.9.0? Ok, ok, I exagerate, but you get the idea.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Travis P <sv...@castle.fastmail.fm>.
On Mar 18, 2004, at 7:45 AM, D.J. Heap wrote:
> Nuutti Kotivuori wrote:
> [snip]
>> No, I don't think you have. I just haven't seen any project where
>> deprecated functions would have not disappeared in a following major
>> release, but instead become the supported functions again. Also, I
>> don't believe I have seen the something2 naming for new functions
>> anywhere. So I would not call those methods conventional. But that may
>> ofcourse be just me.
>> -- Naked
>
> FWIW, the something2 naming is all over the place in MS API's --
> especially COM interfaces. They also used to use somethingEx, but
> mostly stopped doing that as there is nowhere to go after
> that...somethingExEx?
>
> Hardly makes it a standard, of course, but at least it's not
> unprecedented.
>
> DJ
It's certainly not unprecedented. Using Apple's Toolbox library on
GS/OS (Apple IIgs) circa 1987, NewWindow2 was introduced with much more
functionality than NewWindow. I would not be surprised if the
Macintosh Toolbox had the same type of thing though I wasn't
programming Macs at that time but the GS/OS Toolbox was clearly highly
related to the Mac version. Both continued to be supported.
-Travis
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by "D.J. Heap" <dj...@shadyvale.net>.
Nuutti Kotivuori wrote:
[snip]
>
> No, I don't think you have. I just haven't seen any project where
> deprecated functions would have not disappeared in a following major
> release, but instead become the supported functions again. Also, I
> don't believe I have seen the something2 naming for new functions
> anywhere. So I would not call those methods conventional. But that may
> ofcourse be just me.
>
> -- Naked
>
FWIW, the something2 naming is all over the place in MS API's --
especially COM interfaces. They also used to use somethingEx, but
mostly stopped doing that as there is nowhere to go after
that...somethingExEx?
Hardly makes it a standard, of course, but at least it's not unprecedented.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
Greg Hudson wrote:
> It's not really fair to call our approach "unconventional" when
> there is no convention to follow. (Certainly there's none that I'm
> aware of.)
Ah, a mere linguistic mistake on my part I guess. I just meant that it
was not conventional, not that it would be going against a convention.
And you are right, there probably isn't a convention on how such
things are done with programs abiding to the APR versioning document,
since it is stricter than many others.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2004-03-18 at 08:16, Nuutti Kotivuori wrote:
> No, I don't think you have. I just haven't seen any project where
> deprecated functions would have not disappeared in a following major
> release, but instead become the supported functions again. Also, I
> don't believe I have seen the something2 naming for new functions
> anywhere. So I would not call those methods conventional. But that may
> ofcourse be just me.
It's not really fair to call our approach "unconventional" when there is
no convention to follow. (Certainly there's none that I'm aware of.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
Greg Stein wrote:
> It isn't "simply too great". It just isn't possible. If you compile
> against 1.1.x, then your application *requires* 1.1.x. There isn't
> anything that you can do unless you lift the hood and dig very very
> hard to ensure that your app can bind/work against 1.0.x.
>
> The simplest example is a #define constant changing from 1.0.x to
> 1.1.x. That changing value can then make a particular function
> alter its behavior. It would know whether a 1.0.x or a 1.1.x user is
> calling it. This kind of change in the constant is acceptable under
> the guidelines.
Um, the APR versioning text seems to explicitly disallow changes to
constants. It also says "Further, if an application is designed to
work with an earlier minor version, then we don't want to suddenly
change the requirements for that application. This means that the
headers cannot silently map an old function into a newer function, as
that would turn an application, say, based on 1.2 into an application
requiring the 1.4 or later release." And even more "It is possible to
design a set of headers where a macro will always refer to the
'latest' function available. Of course, if an application chooses to
use this macro, then the resulting compiled-binary will be dependent
upon whatever version it was compiled against. This strategy adds the
new functionality for applications, yet retains the necessary source
and binary compatibility for applications designed or built against
previous minor releases."
I don't know, maybe I am just reading the guidelines through my own
set of tinted glass, but I read the minor version constraints as such:
- Application coded originally against 1.0 headers is source and
binary compatible against later minor versions, but also that
application compiled against 1.1 headers is still binary compatible
with 1.0 libraries, if there are no library versioning issues on the
OS level.
- Application coded against 1.1 headers, but without using any 1.1
specific features, is still source and binary compatible against 1.0,
regardless whether it has been compiled against the 1.1 headers or
1.0 headers.
If this is *not* so, then obviously all that I have been talking here
is about princesses and toads.
> Something built against 1.0.x must continue to function with the
> 1.1.x libraries (and the guidelines are defined to enable this), but
> the reverse does not hold true. If you install a 1.1.x application,
> then the libraries must be upgraded to 1.1.x. The theory here is
> that an administrator can feel safe doing the upgrade because it
> will not break any existing 1.0.x applications.
The upgrade mentality is sound, yes. But I always saw that applying to
only applications that actually use the 1.1 specific features, and not
just being compiled or coded against 1.1.
>> Well, I still consider the approach here unconventional, but by no
>> means unsound. So as long as it is documented, it's all rose petals
>> and vanilla ice-cream.
>
> Unconventional? What aspect? I must have missed an earlier post of
> yours :-)
No, I don't think you have. I just haven't seen any project where
deprecated functions would have not disappeared in a following major
release, but instead become the supported functions again. Also, I
don't believe I have seen the something2 naming for new functions
anywhere. So I would not call those methods conventional. But that may
ofcourse be just me.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Mar 17, 2004 at 06:31:55AM +0200, Nuutti Kotivuori wrote:
> kfogel@collab.net wrote:
> > Nuutti Kotivuori <na...@iki.fi> writes:
>
> [...]
>
[ in reference to "avoid 1.1 functions so your code works against 1.0" ]
> > I think if we use your interpretation, the burden on development is
> > simply too great. We've been very clear about what's guaranteed and
> > what's not; let's not impose more burden on ourselves for the sake
> > of an entirely avoidable use case.
It isn't "simply too great". It just isn't possible. If you compile
against 1.1.x, then your application *requires* 1.1.x. There isn't
anything that you can do unless you lift the hood and dig very very hard
to ensure that your app can bind/work against 1.0.x.
The simplest example is a #define constant changing from 1.0.x to 1.1.x.
That changing value can then make a particular function alter its
behavior. It would know whether a 1.0.x or a 1.1.x user is calling it.
This kind of change in the constant is acceptable under the guidelines.
Something built against 1.0.x must continue to function with the 1.1.x
libraries (and the guidelines are defined to enable this), but the reverse
does not hold true. If you install a 1.1.x application, then the libraries
must be upgraded to 1.1.x. The theory here is that an administrator can
feel safe doing the upgrade because it will not break any existing 1.0.x
applications.
>...
> > Sure! We should continue using "@deprecated", because it gives us a
> > well-defined string to search for. But there's no reason the
> > additional documentation can't be present. It can only help. That
Agreed.
> > way if someone *wants* to write compatibly with the 1.0.x API, they
> > can do so, even if only 1.1.x is present on their system.
With the caveats mentioned above. Whatever you compile against will
determine what you must link/run against. If you code to the 1.0.x API and
build against it, then you can run on a system which has 1.x.y libraries
installed.
> On that note, we should probably start using "@since 1.1" (or 1.1.0,
> if that is preferred) on every new function (or thing) added to the
> interface - and probably nuke all those tags at 2.0, since it won't
> matter then anymore and it is just clutter.
Absolutely agreed. ++1.
>...
> > P.S. I've held off putting anything in HACKING about this until the
> > thread reaches some sort of obvious consensus :-).
>
> Well, I still consider the approach here unconventional, but by no
> means unsound. So as long as it is documented, it's all rose petals
> and vanilla ice-cream.
Unconventional? What aspect? I must have missed an earlier post of yours :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"Sander Striker" <st...@apache.org> writes:
> I would think so. It would make much sense to have a foo2() at some
> point, but not a foo(). The whole point of versioning is to not
> have versions in your function names I would think ;).
I assume you meant "would not make much sense".
> > IMHO, we should do this. By resetting our APIs back to the
> > names-of-first-choice, we avoid names growing arbitrarily long (think
> > 3.0).
>
> +1.
+1 to your +1 :-).
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Sander Striker <st...@apache.org>.
> From: kfogel@newton.ch.collab.net [mailto:kfogel@newton.ch.collab.net]On
> Behalf Of kfogel@collab.net
> Greg Hudson <gh...@MIT.EDU> writes:
> > With one set of functions, we're adding foo_depth() variants which
> > include a depth argument.
> >
> > With another set of functions, we didn't want to add foo_pool() variants
> > because that looks dumb, so we're adding foo2() variants instead.
> >
> > I would like us to be consistent.
>
> Good point. A question that impinges on this:
>
> Are we planning to keep these suffixes in 2.0? Or will we change the
> new names to the old names (just keep the new APIs) when we cross the
> 2.0 line?
I would think so. It would make much sense to have a foo2() at some
point, but not a foo(). The whole point of versioning is to not
have versions in your function names I would think ;).
> If we're going to fall back to the old names, then the "2" variants
> might make the most sense.
>
> IMHO, we should do this. By resetting our APIs back to the
> names-of-first-choice, we avoid names growing arbitrarily long (think
> 3.0).
+1.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions
Posted by Nuutti Kotivuori <na...@iki.fi>.
kfogel@collab.net wrote:
> Nuutti Kotivuori <na...@iki.fi> writes:
[...]
> I think if we use your interpretation, the burden on development is
> simply too great. We've been very clear about what's guaranteed and
> what's not; let's not impose more burden on ourselves for the sake
> of an entirely avoidable use case.
"Hmm... Okay, I can relate to that." :-)
>> Simply explaining the situation in the documentation will give the
>> users the best grounds on making the decision over using the newer
>> function and forfeiting older minor version compatibility or just
>> sticking with the old - and also giving them an idea what is going
>> to happen in the future. As an example, in the WC administrative
>> directory lock depth case, the documentation string could simply
>> point the user to a respective svn_wc_adm_foo2 function, point out
>> that it was added since 1.1.0 (or development leading to 1.1.0) and
>> that 2.0 will most likely have the svn_wc_adm_foo function with the
>> parameters of svn_wc_adm_foo2.
>
> Sure! We should continue using "@deprecated", because it gives us a
> well-defined string to search for. But there's no reason the
> additional documentation can't be present. It can only help. That
> way if someone *wants* to write compatibly with the 1.0.x API, they
> can do so, even if only 1.1.x is present on their system.
On that note, we should probably start using "@since 1.1" (or 1.1.0,
if that is preferred) on every new function (or thing) added to the
interface - and probably nuke all those tags at 2.0, since it won't
matter then anymore and it is just clutter.
> Using "2" makes sense when it's a straightforward updating of an old
> API whose name was fine. But when the function's meaning is
> changing significantly, or when the old name is agreed to be
> unsatisfactory, then we can simply choose a better name, with no "2"
> suffix.
>
> Either way, the old, deprecated function can refer to the new one.
Agreed.
> P.S. I've held off putting anything in HACKING about this until the
> thread reaches some sort of obvious consensus :-).
Well, I still consider the approach here unconventional, but by no
means unsound. So as long as it is documented, it's all rose petals
and vanilla ice-cream.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Naming of function versions (Was: Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.)
Posted by kf...@collab.net.
Nuutti Kotivuori <na...@iki.fi> writes:
> Now, I am not saying that deprecating the non-'2' variants of the
> functions would be strictly wrong. There are many ways to look at the
> matter and many ways to view deprecation - but these are my views on
> the matter.
>
> To me, deprecation an awfully strong tag when placed on a function. It
> is something that should chime a warning immediately. It is like
> saying that "You should not be using this function."
That's the idea. We want people writing new code to use the new API,
unless they're explicitly trying to be compatible with older
Subversions.
> But there is a reason to keep on using that function. Considering the
> APR versioning guidelines, it is quite okay to write code against 1.1
> headers that works okay on 1.0 if you don't use any of the new
> features provided by the 1.1 API. Hence everything the 1.1 API
> provides should be just that - additional functions that you can use
> if you want to take advantage of the *new* features, knowing that you
> are then dependant on that minor version.
Anyone who wants/expects their code to compile against 1.0.x needs to
test it with 1.0.x. The whole reason we have these guidelines is so
people will know what's promised and what's not. We didn't promise
that writing against the 1.1 API will automatically work with 1.0.
I think if we use your interpretation, the burden on development is
simply too great. We've been very clear about what's guaranteed and
what's not; let's not impose more burden on ourselves for the sake of
an entirely avoidable use case.
The problem you describe can be solved by simply documenting the facts
(as you recommend yourself, a bit farther on in your post...).
> And like already mentioned, deprecation usually means that function
> will be removed in the near future. Specifying that this only means
> the current form of the function, and not that the function name is
> going away, is confusing to people who use the library.
I grant that it's confusing, until one understands that "2.0" means
"full reset". When that happens, we can easily publish a table
showing how to adjust calling code.
> As a solution to the problem, I propose just being a bit more verbose
> and not using the deprecated tag.
>
> Simply explaining the situation in the documentation will give the
> users the best grounds on making the decision over using the newer
> function and forfeiting older minor version compatibility or just
> sticking with the old - and also giving them an idea what is going to
> happen in the future. As an example, in the WC administrative
> directory lock depth case, the documentation string could simply point
> the user to a respective svn_wc_adm_foo2 function, point out that it
> was added since 1.1.0 (or development leading to 1.1.0) and that 2.0
> will most likely have the svn_wc_adm_foo function with the parameters
> of svn_wc_adm_foo2.
Sure! We should continue using "@deprecated", because it gives us a
well-defined string to search for. But there's no reason the
additional documentation can't be present. It can only help. That
way if someone *wants* to write compatibly with the 1.0.x API, they
can do so, even if only 1.1.x is present on their system.
> As for the actual naming issue when a function interface is "improved"
> - it can be argued both ways. Descriptive names such as *_depth point
> out exactly what can be expected of the new functions, which is an
> important point in my opinion. Coming up with descriptive suffixes can
> be hard, though. Then again, naming them *2 underlines the fact that
> they are just newer revisions of the same functions, and there is
> never any question which alternative is the latest and greatest. But
> it is not descriptive at all and people might have a harder time
> deciding if they want to use the older or newer function.
We should decide on a case-by-case basis. (See my comments in issue
#1552, for example.)
Using "2" makes sense when it's a straightforward updating of an old
API whose name was fine. But when the function's meaning is changing
significantly, or when the old name is agreed to be unsatisfactory,
then we can simply choose a better name, with no "2" suffix.
Either way, the old, deprecated function can refer to the new one.
-Karl
P.S. I've held off putting anything in HACKING about this until the
thread reaches some sort of obvious consensus :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Naming of function versions (Was: Re: [PATCH] Re: The new
svn_wc_adm_*_depth functions.)
Posted by Nuutti Kotivuori <na...@iki.fi>.
D. J. Heap wrote:
> After thinking about it some more, I still like the '2' variant idea
> and then resetting them back to the normal name in a major version
> change -- but as a client it does seem a bit confusing. That is,
> the functions that are marked deprecated are not actually going to
> be removed. In fact, they will become the preferred function again
> with a slightly different parameter list and the '2' variants (which
> were not deprecated) will be removed.
>
> I don't have a better idea, but it does seem a little odd to me.
> Maybe just add a more detailed note about this to the doc comments
> or something?
Well, I personally don't like naming functions with a '2' suffix too
much, but regardless of that, I think the deprecation needs a bit more
thought.
I will quote a few paragraphs from the APR versioning document for
reference:
,----[ Replacing functions ]
| This gets a bit trickier. The original function must remain
| available at the link-level so that an application compiled
| against a minor version will continue to work with later minor
| versions. Further, if an application is designed to work with an
| earlier minor version, then we don't want to suddenly change the
| requirements for that application. This means that the headers
| cannot silently map an old function into a newer function, as that
| would turn an application, say, based on 1.2 into an application
| requiring the 1.4 or later release.
|
| This means that functions cannot truly be replaced. The new,
| alternate function can be made available in the header and
| applications can choose to use it (and become dependent upon the
| minor release where the function appears).
|
| It is possible to design a set of headers where a macro will
| always refer to the "latest" function available. Of course, if an
| application chooses to use this macro, then the resulting
| compiled-binary will be dependent upon whatever version it was
| compiled against. This strategy adds the new functionality for
| applications, yet retains the necessary source and binary
| compatibility for applications designed or built against previous
| minor releases.
`----
,----[ Deprecating functions ]
| ...
|
| Note that all deprecated functions will be removed at the next major
| version bump.
`----
Now, I am not saying that deprecating the non-'2' variants of the
functions would be strictly wrong. There are many ways to look at the
matter and many ways to view deprecation - but these are my views on
the matter.
To me, deprecation an awfully strong tag when placed on a function. It
is something that should chime a warning immediately. It is like
saying that "You should not be using this function."
But there is a reason to keep on using that function. Considering the
APR versioning guidelines, it is quite okay to write code against 1.1
headers that works okay on 1.0 if you don't use any of the new
features provided by the 1.1 API. Hence everything the 1.1 API
provides should be just that - additional functions that you can use
if you want to take advantage of the *new* features, knowing that you
are then dependant on that minor version.
And like already mentioned, deprecation usually means that function
will be removed in the near future. Specifying that this only means
the current form of the function, and not that the function name is
going away, is confusing to people who use the library.
I would like to use deprecation only for functions that have a strong
point against using them - something like that they will not work as
expected, or have security implications or whatever. Functions that if
Subversion library users find themselves using, they should consider
carefully the alternatives, even if it might mean that they will
require a newer minor version to work after that.
As a solution to the problem, I propose just being a bit more verbose
and not using the deprecated tag.
Simply explaining the situation in the documentation will give the
users the best grounds on making the decision over using the newer
function and forfeiting older minor version compatibility or just
sticking with the old - and also giving them an idea what is going to
happen in the future. As an example, in the WC administrative
directory lock depth case, the documentation string could simply point
the user to a respective svn_wc_adm_foo2 function, point out that it
was added since 1.1.0 (or development leading to 1.1.0) and that 2.0
will most likely have the svn_wc_adm_foo function with the parameters
of svn_wc_adm_foo2.
But most importantly, it still drives the point home and does not make
anyone jumpy over the deprecation issue.
As for the actual naming issue when a function interface is "improved"
- it can be argued both ways. Descriptive names such as *_depth point
out exactly what can be expected of the new functions, which is an
important point in my opinion. Coming up with descriptive suffixes can
be hard, though. Then again, naming them *2 underlines the fact that
they are just newer revisions of the same functions, and there is
never any question which alternative is the latest and greatest. But
it is not descriptive at all and people might have a harder time
deciding if they want to use the older or newer function.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> "D.J. Heap" <dj...@shadyvale.net> writes:
>
>>It looks like an agreement is forming on the '2' variants (it sounds
>>reasonable to me) -- if there are no objections then I will wait
>>another day or so and update the patch to rename the functions with
>>that scheme and submit it for review?
>
>
> Sounds good. Thanks!
After thinking about it some more, I still like the '2' variant idea and
then resetting them back to the normal name in a major version change --
but as a client it does seem a bit confusing. That is, the functions
that are marked deprecated are not actually going to be removed. In
fact, they will become the preferred function again with a slightly
different parameter list and the '2' variants (which were not
deprecated) will be removed.
I don't have a better idea, but it does seem a little odd to me. Maybe
just add a more detailed note about this to the doc comments or something?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> It looks like an agreement is forming on the '2' variants (it sounds
> reasonable to me) -- if there are no objections then I will wait
> another day or so and update the patch to rename the functions with
> that scheme and submit it for review?
Sounds good. Thanks!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
[snip]
>
> If we're going to fall back to the old names, then the "2" variants
> might make the most sense.
>
> IMHO, we should do this. By resetting our APIs back to the
> names-of-first-choice, we avoid names growing arbitrarily long (think
> 3.0).
>
> Thoughts?
It looks like an agreement is forming on the '2' variants (it sounds
reasonable to me) -- if there are no objections then I will wait another
day or so and update the patch to rename the functions with that scheme
and submit it for review?
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
Greg Hudson <gh...@MIT.EDU> writes:
> With one set of functions, we're adding foo_depth() variants which
> include a depth argument.
>
> With another set of functions, we didn't want to add foo_pool() variants
> because that looks dumb, so we're adding foo2() variants instead.
>
> I would like us to be consistent.
Good point. A question that impinges on this:
Are we planning to keep these suffixes in 2.0? Or will we change the
new names to the old names (just keep the new APIs) when we cross the
2.0 line?
If we're going to fall back to the old names, then the "2" variants
might make the most sense.
IMHO, we should do this. By resetting our APIs back to the
names-of-first-choice, we avoid names growing arbitrarily long (think
3.0).
Thoughts?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by John Peacock <jp...@rowman.com>.
Greg Hudson wrote:
> With one set of functions, we're adding foo_depth() variants which
> include a depth argument.
>
> With another set of functions, we didn't want to add foo_pool() variants
> because that looks dumb, so we're adding foo2() variants instead.
>
> I would like us to be consistent.
>
Actually, in my case I would like to make both instances semi-public, with the
wrapper remaining public, which is a slightly different case. I'll go along
with whatever scheme reaches consensus first. ;~)
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Boulevard
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5748
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by Greg Hudson <gh...@MIT.EDU>.
Hi. I'm sorry to step in at this late date with a dumb cosmetic issue,
but I'm watching two list conversations about API enhancements which
seem to be going in different naming directions.
With one set of functions, we're adding foo_depth() variants which
include a depth argument.
With another set of functions, we didn't want to add foo_pool() variants
because that looks dumb, so we're adding foo2() variants instead.
I would like us to be consistent.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
kfogel@collab.net wrote:
> "D.J. Heap" <dj...@shadyvale.net> writes:
>
>>New patch using the '2' style for the new functions.
>
>
> I haven't reviewed the whole thing, but I assume it's just the obvious
> transformation to your original patch, so +1.
Yep.
>
> Enough with the posting and the patches and the hey hey hey, though :-).
>>From r9031, you are a full committer (as per the secret discussions
> among the other full committers, in the smoke-filled room in the
> Undisclosed Location, you know the place).
Thanks!
>
> Obviously, feel free to post anything for review before committing
> anytime you want -- but it's your judgement call now.
>
> -K
Yes, I'm most comfortable doing that except for very trivial changes,
perhaps.
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> New patch using the '2' style for the new functions.
I haven't reviewed the whole thing, but I assume it's just the obvious
transformation to your original patch, so +1.
Enough with the posting and the patches and the hey hey hey, though :-).
[PATCH] Re: The new svn_wc_adm_*2 functions.
Posted by "D.J. Heap" <dj...@shadyvale.net>.
New patch using the '2' style for the new functions.
Log:
Rename the new *_depth functions, update call sites (and some
comments), and mark the old functions as deprecated.
* subversion\include\svn_wc.h
Rename new svn_wc_adm_*_depth functions to svn_wc_adm_*2 and
mark the old functions as deprecated.
* subversion\libsvn_wc\wc.h
* subversion\libsvn_wc\copy.c
* subversion\libsvn_wc\log.c
* subversion\libsvn_wc\adm_ops.c
* subversion\libsvn_wc\status.c
* subversion\libsvn_wc\adm_files.c
* subversion\libsvn_wc\lock.c
* subversion\libsvn_wc\update_editor.c
* subversion\libsvn_client\relocate.c
* subversion\libsvn_client\switch.c
* subversion\libsvn_client\delete.c
* subversion\libsvn_client\externals.c
* subversion\libsvn_client\export.c
* subversion\libsvn_client\revisions.c
* subversion\libsvn_client\status.c
* subversion\libsvn_client\prop_commands.c
* subversion\libsvn_client\url.c
* subversion\libsvn_client\checkout.c
* subversion\libsvn_client\revert.c
* subversion\libsvn_client\diff.c
* subversion\libsvn_client\copy.c
* subversion\libsvn_client\log.c
* subversion\libsvn_client\update.c
* subversion\libsvn_client\resolved.c
* subversion\libsvn_client\commit_util.c
* subversion\libsvn_client\add.c
* subversion\libsvn_client\commit.c
* subversion\clients\cmdline\switch-cmd.c
* subversion\clients\cmdline\main.c
* subversion\clients\cmdline\propedit-cmd.c
* subversion\clients\cmdline\info-cmd.c
Update svn_wc_adm_* calls and comments to the new depth aware
versions of those functions.
Re: [PATCH] Re: The new svn_wc_adm_*_depth functions.
Posted by kf...@collab.net.
"D.J. Heap" <dj...@shadyvale.net> writes:
> The latest iteration...
>
> Change calls from the old svn_wc_adm_* functions to the new
> svn_wc_adm_*_depth functions and mark the old functions as deprecated.
+1 to commit. I reviewed every flag change; if there's anything wrong
here, we both missed it :-).
Thanks for doing this cleanup! I've filed issue #1783 as a reminder
to removed deprecated APIs for 2.0. (You don't need to add anything
to the issue, I'm just mentioning it so you know it's done.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org