[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 2803 - trunk/subversion/include trunk/subversion/libsvn_wc trunk/subversion/libsvn_client trunk/subversion/tests/clients/cmdline

From: Karl Fogel <kfogel_at_newton.ch.collab.net>
Date: 2002-07-30 21:30:54 CEST

philip@tigris.org writes:
> --- trunk/subversion/include/svn_wc.h (original)
> +++ trunk/subversion/include/svn_wc.h Tue Jul 30 13:46:17 2002
> @@ -50,6 +50,77 @@
> #endif /* __cplusplus */
>
> +/* Return an access baton in ADM_ACCESS associated with PATH. PATH must be
> + a directory that is locked and part of the set containing the ASSOCIATED
> + access baton.
> +
> + POOL is used only for local processing it is not used for the batons. */
> +svn_error_t *svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access,
> + svn_wc_adm_access_t *associated,
> + const char *path,
> + apr_pool_t *pool);
>
> [...]
>
> +
> +/* Set *LOCKED to non-zero if PATH is locked, else set it to zero. */
> +svn_error_t *svn_wc_locked (svn_boolean_t *locked,
> + const char *path,
> + apr_pool_t *pool);
> +

Minor consistency issue: sometimes you write FOO when documenting a
return-by-reference parameter, and sometimes you say *FOO (for
example, as with "ACCESS_BATON" versus "*LOCKED" above). I believe
our convention -- though not universally followed -- has generally
been to use *FOO.

> +/* ### Should this definition go into lock.c? At present it is visible so
> + ### that users can access the path member, we could provide an access
> + ### function. There is one place that directly access the lock_exists
> + ### member as well. */

Hmmm, yeah. +1 on the opaque type, fwiw.

> --- trunk/subversion/libsvn_wc/lock.c (original)
> +++ trunk/subversion/libsvn_wc/lock.c Tue Jul 30 13:46:26 2002
> @@ -23,38 +23,365 @@
>
> #include "wc.h"
> #include "adm_files.h"
> -
> +#include "svn_pools.h"
>
>
> -svn_error_t *
> -svn_wc_lock (const char *path, int wait_for, apr_pool_t *pool)
> +static svn_error_t *
> +svn_wc__do_adm_close (svn_wc_adm_access_t *adm_access,
> + svn_boolean_t abort);
> +

I think the `abort' should be `preserve_lock', right?

(The name `abort' is causing a "shadows global symbol" warning during
compilation right now.)

Also, no need for the `svn_wc__' prefix on file-static functions. But
they *do* need doc strings :-).

Btw, we haven't generally done advance prototyping like this, except
where absolutely necessary. Any objection to just moving things
around so you don't need the extra prototype? That way we can have
the doc string right above the function definition, which is ideal for
non-public functions, instead of having to choose between the advance
prototype and the function definition.

> +static svn_error_t *
> +svn_wc__lock (svn_wc_adm_access_t *adm_access, int wait_for, apr_pool_t *pool)

Same about prefix and doc string.

> +static svn_error_t *
> +svn_wc__unlock (const char *path, apr_pool_t *pool)
> {
> return svn_wc__remove_adm_file (path, pool, SVN_WC__ADM_LOCK, NULL);
> }

Same...

I'll stop listing these; they're easy for you to find.

> --- trunk/subversion/tests/clients/cmdline/copy_tests.py (original)
> +++ trunk/subversion/tests/clients/cmdline/copy_tests.py Tue Jul 30 13:46:29 2002
> @@ -467,6 +467,39 @@
> return 0
>
>
> +# Takes out working-copy locks for A/B2 and child A/B2/E. At one stage
> +# during issue 749 the second lock cause an already-locked error.
> +def copy_modify_commit(sbox):
> + "copy a directory hierarchy and modify before commit"

What's this? A regression test? You rock so hard it makes my
earlobes tingle.

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 30 21:45:35 2002

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.