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

Re: svn commit: r31511 - in branches/tree-conflicts: . subversion/include subversion/libsvn_wc subversion/svn subversion/tests/libsvn_wc

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 29 May 2008 14:14:55 +0100

Stefan Sperling wrote:
> On Thu, May 29, 2008 at 03:57:50AM -0700, julianfoad_at_tigris.org wrote:
>
>>Author: julianfoad
>>Date: Thu May 29 03:57:50 2008
>>New Revision: 31511
>>
>>Log:
>>Move the code that generates human-readable and XML descriptions of tree
>>conflicts up from libsvn_wc to the client, as libsvn_wc need not know how
>>this information should be presented to the users.
>>
>>* subversion/include/svn_wc.h
>> (svn_wc_conflicted_p2): Add some to-do comments and questions to the doc
>> string.
>> (svn_wc_append_human_readable_tree_conflict_description,
>> svn_wc_append_tree_conflict_info_xml):
>> Move to subversion/svn/tree-conflicts.h, changing the prefix svn_wc_ to
>> svn_cl__.
>
> The functions where public before, and now they aren't. Why that?

Actually that wasn't a very conscious change on my part, but there is
nevertheless a good reason.

At the moment, I'm not seeing any reason to make them public. If another client
wants to provide the exact same descriptions, they can ask us to make these
public, and we will, but until then it's premature to make APIs public just
because there might theoretically be someone else who wants to use them one day.

Better not to expose APIs until needed, because the maintenance burden on a
public API is much higher than on a private one.

> Are you going to add another public API that calls these, or
> is there already an equivalent public API in libsvn_client?

No.

>>* subversion/svn/tree-conflicts.c,
>> subversion/svn/tree-conflicts.h
>> New files with content moved out of libsvn_wc.
>> (svn_cl__append_tree_conflict_info_xml): Tweak to hard-code the XML element
>> and attribute names ("dir", "file", "merge", "deleted", etc.) instead of
>> using the words defined to be used in the WC entries file format.
>
> Mmmh. I'd like it much better if there was a single place where these
> are defined. Oh well, I guess they won't change anymore in the life time
> of libsvn_wc-1 anyway.

Again, its two reasons: first, I noticed that this would have to be accessing
private constants in libsvn_wc, and wondered what to do about that. Then I
thought about it and realised there is no reason why the keywords used in the
XML output should match the data format of the WC entries file. Yes, they
happen to match at the moment, but that is just because the same words sound
right in both places, not because there is any reason that they should match.

In particular, consider that we might want to change the entries file format.
Why should that, a private implementation detail of libsvn_wc, affect the
command-line client's XML output? It shouldn't.

>>* subversion/tests/libsvn_wc/tree-conflict-human-readable-test.c
>> Delete this file, as human-readable descriptions are no longer in libsvn_wc.
>> (I don't feel it's worth transferring these tests to the replacement
>> functions.)
>
> OK, given that you wanted to change that output anyway.

Sorry if this feels like I'm overriding some of your decisions. Let me know if
my reasoning makes sense.

Thanks for reviewing.

- Julian

---------------------------------------------------------------------
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-29 15:15:14 CEST

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