[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: rev 4803 - in trunk/subversion: include libsvn_subr libsvn_ra_local libsvn_client clients/cmdline libsvn_ra_svn libsvn_ra_dav

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2003-02-11 13:00:59 CET

On Monday, February 10, 2003, at 10:30 PM, Greg Stein wrote:

> On Sat, Feb 08, 2003 at 06:59:33PM -0600, rooneg@tigris.org wrote:
>> ...
>> +++ trunk/subversion/libsvn_client/client.h Sat Feb 8 18:59:26 2003
>> ...
>> @@ -225,7 +229,8 @@
>>
>> /* Verify that the path can be deleted without losing stuff, i.e.
>> ensure
>> that there are no modified or unversioned resources under PATH.
>> This is
>> - similar to checking the output of the status command. */
>> + similar to checking the output of the status command. CTX is the
>> client's
>> + context. */
>> svn_error_t * svn_client__can_delete (const char *path,
>> svn_wc_adm_access_t
>> *adm_access,
>> apr_pool_t *pool);
>
> The comment change doesn't match the prototype. There is no CTX
> parameter.
> Rather, the comment doesn't even mention ADM_ACCESS nor POOL.

damn, that was left over from a version of the patch which passed the
context into that function. i'll fix it tonight.

>> ...
>> +++ trunk/subversion/clients/cmdline/main.c Sat Feb 8 18:59:26 2003
>> ...
>> + err = svn_config_read_config (&cfg, pool);
>> + if (err)
>> + svn_handle_error (err, stderr, 0);
>> + else
>> + apr_hash_set (ctx.config, "config", APR_HASH_KEY_STRING, cfg);
>> +
>> + err = svn_config_read_servers (&cfg, pool);
>> + if (err)
>> + svn_handle_error (err, stderr, 0);
>> + else
>> + apr_hash_set (ctx.config, "servers", APR_HASH_KEY_STRING, cfg);
>
> It might be appropriate to have symbolic constants for "config" and
> "servers".

yeah, that seems like a good idea.

>> ...
>> +++ trunk/subversion/libsvn_ra_dav/ra_dav.h Sat Feb 8 18:59:26 2003
>> @@ -35,6 +35,7 @@
>> #include "svn_delta.h"
>> #include "svn_ra.h"
>> #include "svn_dav.h"
>> +#include "svn_config.h"
>
> This include is not necessary. You only added an apr_hash_t to the
> file.

yeah, another left over from a previous version of the patch. will fix.

thanks for the review,

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 11 13:01:47 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.