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

Re: [PATCH] add adm_access to svn_client_commit_item3_t

From: Hyrum K. Wright <hyrum_at_hyrumwright.org>
Date: Tue, 23 Jun 2009 09:51:16 -0500

Thanks for the patched and your continued work as part of Summer of
Code. In the future, could you make sure your patches are attached
with the correct mime-type, as requested by HACKING? Using a mime-
type of text/x-diff, text/x-patch, or text/plain makes it possible to
view and review the patch inline in a mail reader, and increases the
likelihood it will get good review.

A few comments:
I see from the patch you are adding a field to
svn_client_commit_item3_t. It wasn't obvious before r38160, but
adding new members of a public structure should be done at the end of
the structure. You should also update any constructors or copy
functions to account for the new member.

I also noticed that you used svn_wc_adm_access_t. While that's the
traditional approach, we're in the midst of reworking the working copy
library and deprecating the use of svn_wc_adm_access_t in favor of
svn_wc_context_t. Since a svn_wc_context_t isn't tied to a particular
working copy or working copy directory, it won't work as a way to find
your way back home to the source working copy.

(Given this information, part of me wonders how the split of
functionality between libsvn_wc and libsvn_client may change with wc-
ng, but that's a conversation for another day.)


On Jun 23, 2009, at 7:33 AM, HuiHuang wrote:

> <svn_client.patch>

Received on 2009-06-23 16:51:34 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.