Karl Fogel wrote on Mon, 12 May 2008 at 14:31 -0400:
> Daniel Shahaf <d.s_at_daniel.shahaf.co.il> writes:
> >> As long as you're adjusting the doc string, you might as well say what
> >> error is returned if the property value is not valid.
> >
> > Okay. I suppose the existing SVN_ERR_REPOS_BAD_ARGS should be untouched
> > for compatibility, though.
>
> Oh, it's fine to return different errors for different circumstances.
>
I don't know what I meant to say here.
> >> Hmmm, doesn't this imply a change to svn_repos_fs_change_node_prop()'s
> >> doc string? :-)
> >
> > What would you like to change?
>
> The fact that the value is now validated means that the function may
> return errors in circumstances where it formerly would not.
>
> Oh, I see what you mean: it already stated that it is a "validating
> wrapper", so technically, we're not changing its promise. You're right,
> but only because the promise was overly vague before :-).
>
> Now that you're making this change, it's a good chance to increase the
> specificity of the public doc string. That's what I should have said
> before, sorry.
>
Oh, +1. Is there a way to share doxygen documentation between several
functions? Or shall I just repeat the description of the validations
performed, in each of the three docstrings?
Two of them are here:
/* ---------------------------------------------------------------*/
/* Prop-changing wrappers for libsvn_fs routines. */
/* NOTE: svn_repos_fs_change_rev_prop() also exists, but is located
above with the hook-related functions. */
/** Validating wrapper for svn_fs_change_node_prop() (which see for
* argument descriptions).
*/
svn_error_t *
svn_repos_fs_change_node_prop(svn_fs_root_t *root,
const char *path,
const char *name,
const svn_string_t *value,
apr_pool_t *pool);
/** Validating wrapper for svn_fs_change_txn_prop() (which see for
* argument descriptions).
*/
svn_error_t *
svn_repos_fs_change_txn_prop(svn_fs_txn_t *txn,
const char *name,
const svn_string_t *value,
apr_pool_t *pool);
/** Validating wrapper for svn_fs_change_txn_props() (which see for
* argument descriptions).
*
* @since New in 1.5.
*/
svn_error_t *
svn_repos_fs_change_txn_props(svn_fs_txn_t *txn,
apr_array_header_t *props,
apr_pool_t *pool);
/** @} */
> >> ...and this a change to svn_repos_fs_change_txn_props()'s doc string?
> >
> > What would you like to change?
>
> (Same as above.)
>
> >> ...and this a change to svn_repos_fs_change_rev_prop3()'s doc string?
> >
> > Yes, but only because it doesn't already state that it was validating
> > the property.
>
> Saw the change -- thanks.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-12 21:04:16 CEST