[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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/

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 17 Mar 2011 15:52:43 -0400

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

This is an archived mail posted to the Subversion Dev mailing list.