Re: [PATCH] svn_wc_add3(): handles depth on add (was Re: [PATCH] Fix --depth behavior for svn add)

On Sun, May 04, 2008 at 09:26:33PM +0800, Rui, Guo wrote:
> On Mon, Apr 28, 2008 at 05:57:52PM -0400, Karl Fogel wrote:
> >
> > Basically, I think just adding depth to the regular add API is okay.
> >
> Here comes the patch that introduce a new svn_wc_add3() to handle depth on add
> correctly.

Hey Rui,

I spotted on thing:

> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 31014)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -1377,8 +1377,9 @@
>
>
> svn_error_t *
> -svn_wc_add2(const char *path,
> +svn_wc_add3(const char *path,
> svn_wc_adm_access_t *parent_access,
> + svn_depth_t depth,
> const char *copyfrom_url,
> svn_revnum_t copyfrom_rev,
> svn_cancel_func_t cancel_func,
> @@ -1408,6 +1409,11 @@
> _("Unsupported node kind for path '%s'"),
> svn_path_local_style(path, pool));
>
> + /* default to depth_infinity if the value is invalid */
> + if (!(depth >= svn_depth_empty
> + && depth <= svn_depth_infinity))
> + depth = svn_depth_infinity;
> +
> /* Get the original entry for this path if one exists (perhaps
> this is actually a replacement of a previously deleted thing).

Shouldn't this throw an error instead of silently overriding whatever
the caller passed in? Silently overriding the caller's value may produce
subtle bugs in client software that are difficult to track down for our
API consumers. I think it's better to let people know right away when they
did something wrong, by throwing an error at them. You could create
a new error code if no suitable one already eixsts, for example
SVN_ERR_WC_INVALID_DEPTH, or something like that.

-- 
Stefan Sperling <stsp_at_elego.de>                    Software Monkey
 
German law requires the following banner :(
elego Software Solutions GmbH                            HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 
13355 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                               CEO: Olaf Wagner
 
Store password unencrypted (yes/no)? No