> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: dinsdag 23 juni 2009 16:11
> To: yellow.flying
> Cc: dev
> Subject: Re: [PATCH]svn_wc__find_wc_root
>
> On Tue, Jun 23, 2009 at 08:22:19PM +0800, yellow.flying wrote:
> > Hey Stefan,
> >
> > log message:
> >
> > [[[
> >
> > * subversion/include/private/svn_wc_private.h
> > (svn_wc__find_wc_root): new API. Given a path, it return
> > the working copy root path.
> > * subversion/libsvn_wc/update_editor.c
> > ((svn_wc__find_wc_root): new function. It is implement of
> > API svn_wc__find_wc_root in svn_wc_private.h
> > ]]]
>
>
> Nice patch, I've reviewed it, see below.
>
> Index: subversion/include/private/svn_wc_private.h
> ===================================================================
> --- subversion/include/private/svn_wc_private.h (°æ±¾ 38138)
> +++ subversion/include/private/svn_wc_private.h (¹¤×÷¸±±¾)
> @@ -219,6 +219,12 @@
> svn_wc_adm_access_t *adm_access,
> apr_pool_t *pool);
>
> +/** Given @a *path, find its working copy root path @a **wc_root_path
> */
> +svn_error_t *
> +svn_wc__find_wc_root(const char **wc_root_path,
> + const char *path,
> + apr_pool_t *pool);
> +
>
> In this docstring, you should also mention what pool is used for.
> Is pool used for temporary allocations? Only for allocation of the
> result?
> For example:
>
> Use @a pool for temporary allocations.
>
> Or:
>
> Allocate the result in @a pool.
>
> Your new function needs both, so consider passing two pool arguments,
> like this:
>
> svn_wc__find_wc_root(const char **wc_root_path,
> const char *path,
> apr_pool_t *result_pool,
> apr_pool_t *scratch_pool);
>
> Then the docstring should say:
>
> /** Given @a *path, find its working copy root path @a **wc_root_path
> Allocate the result in @a result_pool.
> Use @a scratch_pool for temporary allocations. */
>
>
> Also, the docstring should explain what happens if path is not
> a working copy path (for example "D:\TEMP").
Another useful addition would be what this function considers to be a
working copy root. The WC library uses many concepts for what a working copy
root might be.
* A path with a different url than the parent directory ( = +-
svn_wc_is_root() (Used by update))
* The shortest path of a given path containing the same working copy (= +-
svn_wc__strictly_is_wc_root() (tree conflicts))
* The top level parent of a paths working copy (using absolute path
internally). (WC-NG).
This last version will probably be the directory containing the WC-NG
database.
For future reference it would also be nice if the function documents how it
behaves on non-absolute paths.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2364502
Received on 2009-06-23 16:39:27 CEST