Re: [PATCH] Re: server-side enforcement of svn:date format?

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.

>> > @@ -169,7 +191,7 @@ svn_repos_fs_change_node_prop(svn_fs_root_t *root,
>> > apr_pool_t *pool)
>> > {
>> > /* Validate the property, then call the wrapped function. */
>> > - SVN_ERR(validate_prop(name));
>> > + SVN_ERR(validate_prop(name, value, pool));
>> > return svn_fs_change_node_prop(root, path, name, value, pool);
>> > }
>>
>> 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.

>> ...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 20:31:42 CEST