You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/08/28 03:44:38 UTC
An unexpected effect of new 'deprecated' tags.
The new 'deprecated' tags produce a slew of warnings in our build now.
That's great -- in some places we're still using deprecated functions,
and those places need to be fixed.
But even after we fix them all, there will still be warnings from the
compatibility wrapper functions. For example, imagine these:
svn_foo_bar5()
svn_foo_bar4()
svn_foo_bar3()
svn_foo_bar2()
svn_foo_bar()
How would we usually implement that compatibility stack? Like this:
svn_foo_bar5() { ...; }
svn_foo_bar4() { return svn_foo_bar5(); }
svn_foo_bar3() { return svn_foo_bar4(); }
svn_foo_bar2() { return svn_foo_bar3(); }
svn_foo_bar() { return svn_foo_bar2(); }
The discerning reader will spot the problem immediately :-).
Now, what do we do to avoid being spammed with compilation warnings
for the rest of our lives? Just always reimplement everything in
terms of the newest function? Ick. Or is there some way to mark
calls as known-and-okay, so they won't cause a compilation warning?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Thu, Aug 28, 2008 at 3:18 AM, Philip Martin <ph...@codematters.co.uk> wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>
>> In the meantime, I don't know of a better solution than just choosing
>> whether to see all of them or none of them.
>
> None of them in my view.
>
> I still think the implementation is wrong:
>
> /** Macro used to mark deprecated functions.
> *
> * @since New in 1.6.
> */
> #ifndef SVN_DEPRECATED
> #if !defined(SWIGPERL) && !defined(SWIGPYTHON) && !defined(SWIGRUBY)
> #if defined(__GNUC__) && (__GNUC__ >= 4 || (__GNUC__==3 && __GNUC_MINOR__>=1))
> #define SVN_DEPRECATED __attribute__((deprecated))
> #elif defined(_MSC_VER) && _MSC_VER >= 1300
> #define SVN_DEPRECATED __declspec(deprecated)
> #else
> #define SVN_DEPRECATED
> #endif
> #else
> #define SVN_DEPRECATED
> #endif
> #endif
>
> I think it's wrong for these warnings to be enabled by default in 3rd
> party builds. As in my earlier mail I think the code should look
> like:
>
> #ifdef SVN_DEPRECATED_ENABLE
> #define SVN_DEPRECATED magic
> #else
> #define SVN_DEPRECATED
> #endif
>
> and we arrange for configure to define SVN_DEPRECATED_ENABLE in our
> build, although I'd rather not see spurious warnings by default.
+1 to all of the above.
(I'd advocate that the deprecation warnings would only make sense in
maintainer mode, but YMMV.) -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Philip Martin <ph...@codematters.co.uk>.
Julian Foad <ju...@btopenworld.com> writes:
> In the meantime, I don't know of a better solution than just choosing
> whether to see all of them or none of them.
None of them in my view.
I still think the implementation is wrong:
/** Macro used to mark deprecated functions.
*
* @since New in 1.6.
*/
#ifndef SVN_DEPRECATED
#if !defined(SWIGPERL) && !defined(SWIGPYTHON) && !defined(SWIGRUBY)
#if defined(__GNUC__) && (__GNUC__ >= 4 || (__GNUC__==3 && __GNUC_MINOR__>=1))
#define SVN_DEPRECATED __attribute__((deprecated))
#elif defined(_MSC_VER) && _MSC_VER >= 1300
#define SVN_DEPRECATED __declspec(deprecated)
#else
#define SVN_DEPRECATED
#endif
#else
#define SVN_DEPRECATED
#endif
#endif
I think it's wrong for these warnings to be enabled by default in 3rd
party builds. As in my earlier mail I think the code should look
like:
#ifdef SVN_DEPRECATED_ENABLE
#define SVN_DEPRECATED magic
#else
#define SVN_DEPRECATED
#endif
and we arrange for configure to define SVN_DEPRECATED_ENABLE in our
build, although I'd rather not see spurious warnings by default.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Julian Foad <ju...@btopenworld.com>.
On Thu, 2008-08-28 at 09:27 +0100, David O'Shea wrote:
> On 28/08/2008 04:44, Karl Fogel wrote:
> > Now, what do we do to avoid being spammed with compilation warnings
> > for the rest of our lives? Just always reimplement everything in
> > terms of the newest function? Ick. Or is there some way to mark
> > calls as known-and-okay, so they won't cause a compilation warning?
>
> You can use the "-Wno-deprecated-declarations" option in GCC to suppress
> these warnings. I'm not sure about other compilers though...
That's fine as an all-or-none approach, and we might want to turn them
off by default, but what we really want is for the warnings to be
suppressed inside deprecated functions.
According to GCC's docs, where __attribute__((deprecated)) appears on a
data TYPE definition, GCC SUPPRESSES the warning where a deprecated type
is used inside another deprecated type definition.
I have no idea why GCC doesn't behave the same way w.r.t. functions.
Perhaps another version of the compiler will do what we want. Perhaps
the Microsoft compiler does behave that way.
In the meantime, I don't know of a better solution than just choosing
whether to see all of them or none of them.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by David O'Shea <da...@s3group.com>.
On 28/08/2008 04:44, Karl Fogel wrote:
> Now, what do we do to avoid being spammed with compilation warnings
> for the rest of our lives? Just always reimplement everything in
> terms of the newest function? Ick. Or is there some way to mark
> calls as known-and-okay, so they won't cause a compilation warning?
You can use the "-Wno-deprecated-declarations" option in GCC to suppress
these warnings. I'm not sure about other compilers though...
Regards,
David.
--
The information contained in this e-mail and in any attachments is confidential and is designated solely for the attention of the intended recipient(s). If you are not an intended recipient, you must not use, disclose, copy, distribute or retain this e-mail or any part thereof. If you have received this e-mail in error, please notify the sender by return e-mail and delete all copies of this e-mail from your computer system(s).
Please direct any additional queries to: communications@s3group.com.
Thank You.
Silicon and Software Systems Limited. Registered in Ireland no. 378073.
Registered Office: South County Business Park, Leopardstown, Dublin 18
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Karl Fogel <kf...@red-bean.com>.
Eric Gillespie <ep...@pretzelnet.org> writes:
> Reimplementing all deprecated functions each time we rev it again
> is downright dangerous. We have NO TESTS of these rewritten
> functions. This is true even if we rewrite each one only once,
> leaving a chain of deprecated functions calling each other.
>
> One such rewritten deprecated function almost shipped in 1.5 in a
> completely non-working (segfault) state if I hadn't found it by
> accident (r31055). I wrote a test for it using the Python
> bindings, which is the easiest approach to unit testing.
> Subversion is mostly tested only in the way that svn(1) calls the
> APIs, and API use by anything else is strictly at own risk.
Note that some of our C test code continues to (deliberately) call
deprecated functions. But I think your point still stands; that's not
something we can depend on in general.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Karl Fogel <kf...@red-bean.com> writes:
> Greg Hudson <gh...@MIT.EDU> writes:
> > On Thu, 2008-08-28 at 10:48 -0500, Hyrum K. Wright wrote:
> >> Reimplementing everything in terms of the latest function introduces a subtle
> >> kind of code duplication that I'd rather avoid. And where there's code
> >> duplication, there's bugs.
> >
> > It might be good to look at some examples.
> >
> > My gut reaction is that if it's more concise to use the
> > next-most-deprecated API instead of the current API, we didn't
> > necessarily improve the API with our changes. But that's just theory.
>
> I tried to do this in the past with some big change, I can't remember
> which. I stopped when people objected, but my memory was that it was
> mostly drudge-work -- not challenging, just some extra work.
Reimplementing all deprecated functions each time we rev it again
is downright dangerous. We have NO TESTS of these rewritten
functions. This is true even if we rewrite each one only once,
leaving a chain of deprecated functions calling each other.
One such rewritten deprecated function almost shipped in 1.5 in a
completely non-working (segfault) state if I hadn't found it by
accident (r31055). I wrote a test for it using the Python
bindings, which is the easiest approach to unit testing.
Subversion is mostly tested only in the way that svn(1) calls the
APIs, and API use by anything else is strictly at own risk.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Karl Fogel <kf...@red-bean.com>.
Greg Hudson <gh...@MIT.EDU> writes:
> On Thu, 2008-08-28 at 10:48 -0500, Hyrum K. Wright wrote:
>> Reimplementing everything in terms of the latest function introduces a subtle
>> kind of code duplication that I'd rather avoid. And where there's code
>> duplication, there's bugs.
>
> It might be good to look at some examples.
>
> My gut reaction is that if it's more concise to use the
> next-most-deprecated API instead of the current API, we didn't
> necessarily improve the API with our changes. But that's just theory.
I tried to do this in the past with some big change, I can't remember
which. I stopped when people objected, but my memory was that it was
mostly drudge-work -- not challenging, just some extra work.
But that was one set of interfaces. I'm not sure it answers the
question of whether wrapping current-API-always would be bug-prone in
general.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2008-08-28 at 10:48 -0500, Hyrum K. Wright wrote:
> Reimplementing everything in terms of the latest function introduces a subtle
> kind of code duplication that I'd rather avoid. And where there's code
> duplication, there's bugs.
It might be good to look at some examples.
My gut reaction is that if it's more concise to use the
next-most-deprecated API instead of the current API, we didn't
necessarily improve the API with our changes. But that's just theory.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Arfrever Frehtes Taifersar Arahesis wrote:
> 2008-08-28 05:44:38 Karl Fogel napisał(a):
>> The new 'deprecated' tags produce a slew of warnings in our build now.
>> That's great -- in some places we're still using deprecated functions,
>> and those places need to be fixed.
>>
>> But even after we fix them all, there will still be warnings from the
>> compatibility wrapper functions. For example, imagine these:
>>
>> svn_foo_bar5()
>> svn_foo_bar4()
>> svn_foo_bar3()
>> svn_foo_bar2()
>> svn_foo_bar()
>>
>> How would we usually implement that compatibility stack? Like this:
>>
>> svn_foo_bar5() { ...; }
>> svn_foo_bar4() { return svn_foo_bar5(); }
>> svn_foo_bar3() { return svn_foo_bar4(); }
>> svn_foo_bar2() { return svn_foo_bar3(); }
>> svn_foo_bar() { return svn_foo_bar2(); }
>>
>> The discerning reader will spot the problem immediately :-).
>>
>> Now, what do we do to avoid being spammed with compilation warnings
>> for the rest of our lives? Just always reimplement everything in
>> terms of the newest function?
>
> +1.
An emphatic -1.
Reimplementing everything in terms of the latest function introduces a subtle
kind of code duplication that I'd rather avoid. And where there's code
duplication, there's bugs.
-Hyrum
Re: An unexpected effect of new 'deprecated' tags.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Stefan Küng wrote:
> One thing that bugged me about the deprecated functions for some time
> now is the doxygen comment. All deprecated functions only have a text
> like "\deprecated: provided for backward compatibility with the 1.x
> API". But it doesn't say which function to use instead.
That's would indeed be a handy bit of knowledge to have, wouldn't it? Good
suggestion!
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Deprecated functions should say what replaces them [was: An unexpected
effect of new 'deprecated' tags.]
Posted by Stefan Küng <to...@gmail.com>.
Julian Foad wrote:
> Stefan Küng wrote:
>> One thing that bugged me about the deprecated functions for some time
>> now is the doxygen comment. All deprecated functions only have a text
>> like "\deprecated: provided for backward compatibility with the 1.x
>> API". But it doesn't say which function to use instead.
>>
>> For most functions, that's not a big problem: only look for a function
>> with the same name but a higher number at the end (e.g.,
>> svn_client_merge3 instead of svn_client_merge2). But there are some
>> functions which don't have that:
>>
>> svn_client_ls3 is deprecated, but there is no svn_client_ls4
>> svn_client_get_* (auth provider functions) also have no 'higher number
>> function'.
>>
>> For svn_client_ls3, the function to use would be svn_client_list2, and
>> for the auth provider functions it would be the svn_auth_get_* functions.
>>
>> A user who doesn't follow the commits has to search the whole docs to
>> find the new function to use.
>>
>> So I suggest to extend those doxygen comments like this:
>> "\deprecated: provided for backward compatibility with the 1.x API. Use
>> svn_client_list() instead."
>>
>> This would make it immediately clear which function superseeds the
>> deprecated one.
>
> +1. Can you send a patch? I think this is only necessary for the
> functions where it is not just a higher number.
Will do. But I have to leave in a few minutes, so the patch will have to
wait until tomorrow.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: Deprecated functions should say what replaces them [was: An
unexpected effect of new 'deprecated' tags.]
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Küng wrote:
> Arfrever Frehtes Taifersar Arahesis wrote:
[...]
> > Maybe s/svn_client_get_commit_log2_t/log_msg_func2/.
> Thanks for the review. New patch attached which contains your improvements.
+1. Go ahead and commit it, Stefan. And thanks.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Deprecated functions should say what replaces them [was: An unexpected
effect of new 'deprecated' tags.]
Posted by Stefan Küng <to...@gmail.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
>> @@ -150,6 +152,7 @@
>> * SVN_AUTH_PARAM_DEFAULT_PASSWORD).
>> *
>> * @deprecated Provided for backward compatibility with the 1.3 API.
>> + * Use svn_auth_get_simple_provider() instead.
>
> svn_auth_get_simple_provider() is also deprecated.
> The comment should suggest to use svn_auth_get_simple_provider2().
>> @@ -242,6 +249,7 @@
>> * certificate is protected by a passphrase.
>> *
>> * @deprecated Provided for backward compatibility with the 1.3 API.
>> + * Use svn_auth_get_ssl_client_cert_pw_file_provider() instead.
>
> svn_auth_get_ssl_client_cert_pw_file_provider() is also deprecated.
> svn_auth_get_ssl_client_cert_pw_file_provider2() should be used.
>
>> @@ -801,20 +812,24 @@
>>
>> /** notification callback function.
>> * This will be called by notify_func2() by default.
>> - * @deprecated Provided for backward compatibility with the 1.1 API. */
>> + * @deprecated Provided for backward compatibility with the 1.1 API.
>> + * Use @c svn_wc_notify_func2_t instead. */
>
> Maybe s/svn_wc_notify_func2_t/notify_func2/.
>
>> svn_wc_notify_func_t notify_func;
>>
>> /** notification callback baton for notify_func()
>> - * @deprecated Provided for backward compatibility with the 1.1 API. */
>> + * @deprecated Provided for backward compatibility with the 1.1 API.
>> + * Use @c notify_baton2 instead */
>> void *notify_baton;
>>
>> /** Log message callback function. NULL means that Subversion
>> * should try not attempt to fetch a log message.
>> - * @deprecated Provided for backward compatibility with the 1.2 API. */
>> + * @deprecated Provided for backward compatibility with the 1.2 API.
>> + * Use @c svn_client_get_commit_log2_t instead. */
>
> Maybe s/svn_client_get_commit_log2_t/log_msg_func2/.
>
Thanks for the review. New patch attached which contains your improvements.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: Deprecated functions should say what replaces them [was: An unexpected effect of new 'deprecated' tags.]
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-08-29 17:09:30 Stefan Küng napisał(a):
> Julian Foad wrote:
> > On Fri, 2008-08-29 at 16:14 +0200, Stefan K�ng wrote:
> >> Julian Foad wrote:
> >>> Stefan K�ng wrote:
> >>>> So I suggest to extend those doxygen comments like this:
> >>>> "\deprecated: provided for backward compatibility with the 1.x API. Use
> >>>> svn_client_list() instead."
> >>>>
> >>>> This would make it immediately clear which function superseeds the
> >>>> deprecated one.
> >>> +1. Can you send a patch? I think this is only necessary for the
> >>> functions where it is not just a higher number.
> >> Patch attached.
> >> If it's ok, I'll commit it.
> >
> > In Doxygen mark-up, to refer to another function, just give the
> > function's name with "()" after it like in your example above, and to
> > refer to another non-function such as a type, use the "@c" prefix. (I
> > think of "@c" as "Cross-reference".) We use "@a" to refer to an Argument
> > of the present function.
> >
> > To refer to a function-type, I would use "@c". From within the
> > documentation of a member of a structure, to refer to another member of
> > the same structure, I would use "@c". (I don't know that these are
> > consistently used conventions.)
> >
> > Other than that, I expect your patch is fine. I haven't gone through and
> > checked that you've specified the right successor in each case, but I
> > would trust that you've got it right or very nearly so. You've got my +1
> > to commit it after the above tweak. You might want to wait a few hours
> > in case abybody else reviews it, but then again you might not and you
> > don't have to! (It can still be reviewed after check-in.)
>
> Thanks for the review!
> I've changed the patch according to your advice. New patch is attached.
> [[[
> Document which function replaces deprecated functions.
> * subversion/include/svn_client.h,
> subversion/include/svn_cmdline.h: add doc string.
> ]]]
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 32809)
> +++ subversion/include/svn_client.h (working copy)
> @@ -104,6 +104,7 @@
> * re-prompt @a retry_limit times (via svn_auth_next_credentials()).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_simple_prompt_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -127,6 +128,7 @@
> * re-prompt @a retry_limit times (via svn_auth_next_credentials()).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_username_prompt_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -150,6 +152,7 @@
> * SVN_AUTH_PARAM_DEFAULT_PASSWORD).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_simple_provider() instead.
svn_auth_get_simple_provider() is also deprecated.
The comment should suggest to use svn_auth_get_simple_provider2().
> */
> SVN_DEPRECATED
> void
> @@ -178,6 +181,7 @@
> * if the password were not cached at all.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_windows_simple_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -195,6 +199,7 @@
> * @c SVN_AUTH_PARAM_DEFAULT_USERNAME).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_username_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -210,6 +215,7 @@
> * security on an error.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_server_trust_file_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -226,6 +232,7 @@
> * client certificate for authentication when requested by a server.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_client_cert_file_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -242,6 +249,7 @@
> * certificate is protected by a passphrase.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_client_cert_pw_file_provider() instead.
svn_auth_get_ssl_client_cert_pw_file_provider() is also deprecated.
svn_auth_get_ssl_client_cert_pw_file_provider2() should be used.
> */
> SVN_DEPRECATED
> void
> @@ -258,6 +266,7 @@
> * SSL security on an error.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_server_trust_prompt_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -277,6 +286,7 @@
> * a server. The prompt will be retried @a retry_limit times.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_client_cert_prompt_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -297,6 +307,7 @@
> * be retried @a retry_limit times.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use svn_auth_get_ssl_client_cert_pw_prompt_provider() instead.
> */
> SVN_DEPRECATED
> void
> @@ -801,20 +812,24 @@
>
> /** notification callback function.
> * This will be called by notify_func2() by default.
> - * @deprecated Provided for backward compatibility with the 1.1 API. */
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + * Use @c svn_wc_notify_func2_t instead. */
Maybe s/svn_wc_notify_func2_t/notify_func2/.
> svn_wc_notify_func_t notify_func;
>
> /** notification callback baton for notify_func()
> - * @deprecated Provided for backward compatibility with the 1.1 API. */
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + * Use @c notify_baton2 instead */
> void *notify_baton;
>
> /** Log message callback function. NULL means that Subversion
> * should try not attempt to fetch a log message.
> - * @deprecated Provided for backward compatibility with the 1.2 API. */
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + * Use @c svn_client_get_commit_log2_t instead. */
Maybe s/svn_client_get_commit_log2_t/log_msg_func2/.
--
Arfrever Frehtes Taifersar Arahesis
Re: Deprecated functions should say what replaces them [was: An unexpected
effect of new 'deprecated' tags.]
Posted by Stefan Küng <to...@gmail.com>.
Julian Foad wrote:
> On Fri, 2008-08-29 at 16:14 +0200, Stefan Küng wrote:
>> Julian Foad wrote:
>>> Stefan Küng wrote:
>>>> So I suggest to extend those doxygen comments like this:
>>>> "\deprecated: provided for backward compatibility with the 1.x API. Use
>>>> svn_client_list() instead."
>>>>
>>>> This would make it immediately clear which function superseeds the
>>>> deprecated one.
>>> +1. Can you send a patch? I think this is only necessary for the
>>> functions where it is not just a higher number.
>> Patch attached.
>> If it's ok, I'll commit it.
>
> In Doxygen mark-up, to refer to another function, just give the
> function's name with "()" after it like in your example above, and to
> refer to another non-function such as a type, use the "@c" prefix. (I
> think of "@c" as "Cross-reference".) We use "@a" to refer to an Argument
> of the present function.
>
> To refer to a function-type, I would use "@c". From within the
> documentation of a member of a structure, to refer to another member of
> the same structure, I would use "@c". (I don't know that these are
> consistently used conventions.)
>
> Other than that, I expect your patch is fine. I haven't gone through and
> checked that you've specified the right successor in each case, but I
> would trust that you've got it right or very nearly so. You've got my +1
> to commit it after the above tweak. You might want to wait a few hours
> in case abybody else reviews it, but then again you might not and you
> don't have to! (It can still be reviewed after check-in.)
Thanks for the review!
I've changed the patch according to your advice. New patch is attached.
I've also checked the doxygen output to make sure the function names are
properly turned into links to the corresponding doc part.
I'll wait a few hours in case someone else wants to review this.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: Deprecated functions should say what replaces them [was: An
unexpected effect of new 'deprecated' tags.]
Posted by Julian Foad <ju...@btopenworld.com>.
On Fri, 2008-08-29 at 16:14 +0200, Stefan Küng wrote:
> Julian Foad wrote:
> > Stefan Küng wrote:
> >> So I suggest to extend those doxygen comments like this:
> >> "\deprecated: provided for backward compatibility with the 1.x API. Use
> >> svn_client_list() instead."
> >>
> >> This would make it immediately clear which function superseeds the
> >> deprecated one.
> >
> > +1. Can you send a patch? I think this is only necessary for the
> > functions where it is not just a higher number.
>
> Patch attached.
> If it's ok, I'll commit it.
In Doxygen mark-up, to refer to another function, just give the
function's name with "()" after it like in your example above, and to
refer to another non-function such as a type, use the "@c" prefix. (I
think of "@c" as "Cross-reference".) We use "@a" to refer to an Argument
of the present function.
To refer to a function-type, I would use "@c". From within the
documentation of a member of a structure, to refer to another member of
the same structure, I would use "@c". (I don't know that these are
consistently used conventions.)
Other than that, I expect your patch is fine. I haven't gone through and
checked that you've specified the right successor in each case, but I
would trust that you've got it right or very nearly so. You've got my +1
to commit it after the above tweak. You might want to wait a few hours
in case abybody else reviews it, but then again you might not and you
don't have to! (It can still be reviewed after check-in.)
Thanks.
- Julian
> [[[
> Document which function replaces deprecated functions.
> * subversion/include/svn_client.h,
> subversion/include/svn_cmdline.h: add doc string.
> ]]]
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 32806)
> +++ subversion/include/svn_client.h (working copy)
> @@ -104,6 +104,7 @@
> * re-prompt @a retry_limit times (via svn_auth_next_credentials()).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_simple_prompt_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -127,6 +128,7 @@
> * re-prompt @a retry_limit times (via svn_auth_next_credentials()).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_username_prompt_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -150,6 +152,7 @@
> * SVN_AUTH_PARAM_DEFAULT_PASSWORD).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_simple_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -178,6 +181,7 @@
> * if the password were not cached at all.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_windows_simple_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -195,6 +199,7 @@
> * @c SVN_AUTH_PARAM_DEFAULT_USERNAME).
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_username_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -210,6 +215,7 @@
> * security on an error.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_server_trust_file_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -226,6 +232,7 @@
> * client certificate for authentication when requested by a server.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_client_cert_file_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -242,6 +249,7 @@
> * certificate is protected by a passphrase.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_client_cert_pw_file_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -258,6 +266,7 @@
> * SSL security on an error.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_server_trust_prompt_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -277,6 +286,7 @@
> * a server. The prompt will be retried @a retry_limit times.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_client_cert_prompt_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -297,6 +307,7 @@
> * be retried @a retry_limit times.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_auth_get_ssl_client_cert_pw_prompt_provider instead.
> */
> SVN_DEPRECATED
> void
> @@ -801,20 +812,24 @@
>
> /** notification callback function.
> * This will be called by notify_func2() by default.
> - * @deprecated Provided for backward compatibility with the 1.1 API. */
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + * Use @a svn_wc_notify_func2_t instead. */
> svn_wc_notify_func_t notify_func;
>
> /** notification callback baton for notify_func()
> - * @deprecated Provided for backward compatibility with the 1.1 API. */
> + * @deprecated Provided for backward compatibility with the 1.1 API.
> + * Use @a notify_baton2 instead */
> void *notify_baton;
>
> /** Log message callback function. NULL means that Subversion
> * should try not attempt to fetch a log message.
> - * @deprecated Provided for backward compatibility with the 1.2 API. */
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + * Use @a svn_client_get_commit_log2_t instead. */
> svn_client_get_commit_log_t log_msg_func;
>
> /** log message callback baton
> - * @deprecated Provided for backward compatibility with the 1.2 API. */
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + * Use @a log_msg_baton2 instead. */
> void *log_msg_baton;
>
> /** a hash mapping of <tt>const char *</tt> configuration file names to
> @@ -2947,6 +2962,7 @@
> * resolution support.
> *
> * @deprecated Provided for backward compatibility with the 1.4 API.
> + * Use @a svn_client_resolve instead.
> */
> SVN_DEPRECATED
> svn_error_t *
> @@ -3937,6 +3953,7 @@
> * @since New in 1.3.
> *
> * @deprecated Provided for backward compatibility with the 1.3 API.
> + * Use @a svn_client_list2 instead.
> */
> SVN_DEPRECATED
> svn_error_t *
> @@ -3955,6 +3972,7 @@
> * @since New in 1.2.
> *
> * @deprecated Provided for backward compatibility with the 1.2 API.
> + * Use @a svn_client_list2 instead.
> */
> SVN_DEPRECATED
> svn_error_t *
> @@ -3971,6 +3989,7 @@
> * the same as @a revision.
> *
> * @deprecated Provided for backward compatibility with the 1.1 API.
> + * Use @a svn_client_list2 instead.
> */
> SVN_DEPRECATED
> svn_error_t *
> Index: subversion/include/svn_cmdline.h
> ===================================================================
> --- subversion/include/svn_cmdline.h (revision 32806)
> +++ subversion/include/svn_cmdline.h (working copy)
> @@ -325,6 +325,7 @@
> *
> * @since New in 1.4.
> * @deprecated Provided for backward compatibility with the 1.5 API.
> + * Use @a svn_cmdline_set_up_auth_baton instead.
> *
> * @note This deprecation does not follow the usual pattern of putting
> * a new number on end of the function's name. Instead, the new
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Deprecated functions should say what replaces them [was: An unexpected
effect of new 'deprecated' tags.]
Posted by Stefan Küng <to...@gmail.com>.
Julian Foad wrote:
> Stefan Küng wrote:
>> One thing that bugged me about the deprecated functions for some time
>> now is the doxygen comment. All deprecated functions only have a text
>> like "\deprecated: provided for backward compatibility with the 1.x
>> API". But it doesn't say which function to use instead.
>>
>> For most functions, that's not a big problem: only look for a function
>> with the same name but a higher number at the end (e.g.,
>> svn_client_merge3 instead of svn_client_merge2). But there are some
>> functions which don't have that:
>>
>> svn_client_ls3 is deprecated, but there is no svn_client_ls4
>> svn_client_get_* (auth provider functions) also have no 'higher number
>> function'.
>>
>> For svn_client_ls3, the function to use would be svn_client_list2, and
>> for the auth provider functions it would be the svn_auth_get_* functions.
>>
>> A user who doesn't follow the commits has to search the whole docs to
>> find the new function to use.
>>
>> So I suggest to extend those doxygen comments like this:
>> "\deprecated: provided for backward compatibility with the 1.x API. Use
>> svn_client_list() instead."
>>
>> This would make it immediately clear which function superseeds the
>> deprecated one.
>
> +1. Can you send a patch? I think this is only necessary for the
> functions where it is not just a higher number.
Patch attached.
If it's ok, I'll commit it.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Deprecated functions should say what replaces them [was: An
unexpected effect of new 'deprecated' tags.]
Posted by Julian Foad <ju...@btopenworld.com>.
Stefan Küng wrote:
> One thing that bugged me about the deprecated functions for some time
> now is the doxygen comment. All deprecated functions only have a text
> like "\deprecated: provided for backward compatibility with the 1.x
> API". But it doesn't say which function to use instead.
>
> For most functions, that's not a big problem: only look for a function
> with the same name but a higher number at the end (e.g.,
> svn_client_merge3 instead of svn_client_merge2). But there are some
> functions which don't have that:
>
> svn_client_ls3 is deprecated, but there is no svn_client_ls4
> svn_client_get_* (auth provider functions) also have no 'higher number
> function'.
>
> For svn_client_ls3, the function to use would be svn_client_list2, and
> for the auth provider functions it would be the svn_auth_get_* functions.
>
> A user who doesn't follow the commits has to search the whole docs to
> find the new function to use.
>
> So I suggest to extend those doxygen comments like this:
> "\deprecated: provided for backward compatibility with the 1.x API. Use
> svn_client_list() instead."
>
> This would make it immediately clear which function superseeds the
> deprecated one.
+1. Can you send a patch? I think this is only necessary for the
functions where it is not just a higher number.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: An unexpected effect of new 'deprecated' tags.
Posted by Stefan Küng <to...@gmail.com>.
Arfrever Frehtes Taifersar Arahesis wrote:
> 2008-08-28 05:44:38 Karl Fogel napisał(a):
>> The new 'deprecated' tags produce a slew of warnings in our build now.
>> That's great -- in some places we're still using deprecated functions,
>> and those places need to be fixed.
>>
>> But even after we fix them all, there will still be warnings from the
>> compatibility wrapper functions. For example, imagine these:
>>
>> svn_foo_bar5()
>> svn_foo_bar4()
>> svn_foo_bar3()
>> svn_foo_bar2()
>> svn_foo_bar()
>>
>> How would we usually implement that compatibility stack? Like this:
>>
>> svn_foo_bar5() { ...; }
>> svn_foo_bar4() { return svn_foo_bar5(); }
>> svn_foo_bar3() { return svn_foo_bar4(); }
>> svn_foo_bar2() { return svn_foo_bar3(); }
>> svn_foo_bar() { return svn_foo_bar2(); }
>>
>> The discerning reader will spot the problem immediately :-).
>>
>> Now, what do we do to avoid being spammed with compilation warnings
>> for the rest of our lives? Just always reimplement everything in
>> terms of the newest function?
One thing that bugged me about the deprecated functions for some time
now is the doxygen comment. All deprecated functions only have a text
like "\deprecated: provided for backward compatibility with the 1.x
API". But it doesn't say which function to use instead.
For most functions, that's not a big problem: only look for a function
with the same name but a higher number at the end (e.g.,
svn_client_merge3 instead of svn_client_merge2). But there are some
functions which don't have that:
svn_client_ls3 is deprecated, but there is no svn_client_ls4
svn_client_get_* (auth provider functions) also have no 'higher number
function'.
For svn_client_ls3, the function to use would be svn_client_list2, and
for the auth provider functions it would be the svn_auth_get_* functions.
A user who doesn't follow the commits has to search the whole docs to
find the new function to use.
So I suggest to extend those doxygen comments like this:
"\deprecated: provided for backward compatibility with the 1.x API. Use
svn_client_list() instead."
This would make it immediately clear which function superseeds the
deprecated one.
Opinions?
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
Re: An unexpected effect of new 'deprecated' tags.
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-08-28 05:44:38 Karl Fogel napisał(a):
> The new 'deprecated' tags produce a slew of warnings in our build now.
> That's great -- in some places we're still using deprecated functions,
> and those places need to be fixed.
>
> But even after we fix them all, there will still be warnings from the
> compatibility wrapper functions. For example, imagine these:
>
> svn_foo_bar5()
> svn_foo_bar4()
> svn_foo_bar3()
> svn_foo_bar2()
> svn_foo_bar()
>
> How would we usually implement that compatibility stack? Like this:
>
> svn_foo_bar5() { ...; }
> svn_foo_bar4() { return svn_foo_bar5(); }
> svn_foo_bar3() { return svn_foo_bar4(); }
> svn_foo_bar2() { return svn_foo_bar3(); }
> svn_foo_bar() { return svn_foo_bar2(); }
>
> The discerning reader will spot the problem immediately :-).
>
> Now, what do we do to avoid being spammed with compilation warnings
> for the rest of our lives? Just always reimplement everything in
> terms of the newest function?
+1.
--
Arfrever Frehtes Taifersar Arahesis