Re: svn commit: r1082658 - in /subversion/trunk/subversion: include/private/ libsvn_client/ libsvn_fs_base/ libsvn_fs_base/util/ libsvn_subr/ libsvn_wc/ tests/cmdline/

On Mar 17, 2011 7:40 PM, <pburba_at_apache.org> wrote:
>
> Author: pburba
> Date: Thu Mar 17 19:40:07 2011
> New Revision: 1082658
>
> URL: http://svn.apache.org/viewvc?rev=1082658&view=rev
> Log:
> Leverage the recent improvements to svn proplist -R[1] so that svn propget
-R
> can do away with svn_wc__node_walk_children().
>
> [1] See r1066541 and r1071283.
>
> * subversion/include/private/svn_skel.h
> * subversion/libsvn_subr/skel.c
> (svn_skel__parse_proplist): Optionally parse only a particular property.

I think this is a truly awful API. You have two entirely different
semantics, keyed by a parameter, rather than two functions with appropriate
param lists. And the "parse one" can just return a string_t rather than a
hash-of-one-element.

Internally, the parse code might look like this, shared between the two, but
exposing it thus way is really ugly.

Could you please revert this change, and redo it with the two-function
approach? (or, of course, explain why that doesn't make sense)

Cheers,
-g
Received on 2011-03-17 20:53:13 CET