[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

Huihuang,
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.)

Best,
-Hyrum

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

> <svn_client.patch>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2364506
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.