[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: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 17 Mar 2011 15:58:37 -0400

On Thu, Mar 17, 2011 at 3:52 PM, Greg Stein <gstein_at_gmail.com> wrote:
> 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)

Hi Greg,

Point taken, I'll redo it with two functions.

Paul

> Cheers,
> -g
>
Received on 2011-03-17 21:00:26 CET

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