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

Re: [PATCH]remove adm_access batons in svn_client__do_commit

From: Stefan Sperling <stsp_at_elego.de>
Date: Thu, 6 Aug 2009 13:37:31 +0100

On Thu, Aug 06, 2009 at 04:25:24PM +0800, yellow.flying wrote:
> sorry, I forget the log message:(
>
> Hey,
>
> As you suggested, I make a new patch which does not include while-space changes.
> After this patch is commited, I will make another patch about format later.
>
> See below: Thank you:)

Great patch, thanks!

I've read over it, and ran the commit regression tests which all
passed. Committed in r38580.

> Log:
> [[[
> Rip out some adm_access usage in svn_client__do_commit().
>
> * subversion/libsvn_client/client.h
> (adm_access): Remove.

This is the name of a parameter, but we put function names or names
of global variables into the ( ). I've tweaked this before commit:

* subversion/libsvn_client/client.h
  (svn_client__do_commit): Remove adm_access parameter.

> * subversion/libsvn_client/commit.c
> (base_dir_access): Remove.

Which base_dir_access does this refer to?
There are many in that file.

Your patch only changed a call to svn_client__do_commit(), which passed
a variable called base_dir_access for the 'adm_access' parameter.
But nowhere did you remove such a variable.

So I've changed this to:

* subversion/libsvn_client/commit.c
  (svn_client_commit4): Track above parameter removal.

> * subversion/libsvn_client/commit_util.c
> (adm_access): Remove.
> (local_abspath): Moved to the head of do_item_commit().
> (tmp_entry): Remove.
> (svn_wc_entry): Remove.
> (svn_wc_transmit_prop_deltas): Use svn_wc_transmit_prop_deltas2 instead.
> (adm_access): Removed from svn_client__do_commit parameters.
> (cb_baton.adm_access): Remove.

Again, function names or names of global variables belong into ( ).

I've rewritten this part as:

* subversion/libsvn_client/commit_util.c
  (do_item_commit): Stop using an adm_access and entries.
   Move an already existing local_abspath to function-wide scope,
   and use it and the wc context provided in the client context
   for various updates from wc-1 APIs to WC-NG APIs.

That's all you need to say, really. Everyone knows that you are updating
to WC-NG in this diff, you don't need to list every single function call
you have changed.

The most important purpose of a log message is to document why and where
something has changed, and to provide a high-level summary of the change.
The details of the change are always in the diff, so you don't need to be
too detailed. But it's not easy to find a good balance between too little
and too much detail -- that just comes over time with practice.

> * subversion/libsvn_client/copy.c
> (adm_access): Remove.

I've combined this with an earlier log message entry, so it now reads:

* subversion/libsvn_client/commit.c,
  subversion/libsvn_client/copy.c
  (svn_client_commit4, wc_to_repos_copy): Track above parameter removal.

Thanks for the patch!
Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2380812
Received on 2009-08-06 14:38:08 CEST

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