Hi Neels.  Brief response.

Looks like a good improvement.

svn_wc__committable_external_info_t: Use the new 'svn_kind_t' instead of 
svn_node_kind_t.

svn_client_commit6(): Could you extract the main chunk of added code as a 
separate function?  That would help me (the reader) quickly understand how much 
of the local state it does *not* touch.

harvest_committables(): Would the 'is_explicit_target' parameter be better 
named something like 'include_file_externals'?  I'm not sure if that would 
better reflect its purpose in that function; I haven't got my head around it 
well enough, so just asking.

svn_client_commit6(): In the doc string, of instead of "If A and B, all file 
respectively dir externals as defined ...", I suggest "If A and/or B, all file 
and/or dir externals (respectively) as defined by ...".  Rationale: 
"respectively" isn't used as a conjunction word in English: 
<http://www.transblawg.eu/index.php?/archives/870-Resp.-and-other-non-existent-English-wordsNicht-existente-englische-Woerter.html>.

In the same doc string: mention the TODO about --depth=immediates skipping dir 
externals, that you documented inside the function.

svn_wc__committable_externals_below(): Document the 'immediates_only' parameter.

- Julian


--- On Thu, 3/11/11, Neels J Hofmeyr <ne...@elego.de> wrote:

> From: Neels J Hofmeyr <ne...@elego.de>
> Subject: [PATCH] commit --include-externals (v2)
> To: "Subversion Development" <dev@subversion.apache.org>, "C. Michael Pilato" 
> <cmpil...@collab.net>, "Bert Huijben" <b...@vmoo.com>
> Date: Thursday, 3 November, 2011, 13:33
> I've rinsed and improved my proposed
> feature dubbed
>   svn commit --include-externals
> (Related issues: #1167, #3563)
> 
> I hope this will cut a much clearer path through the jungle
> that is
> externals behavior. Now I'm hoping for some reviews!
> 
> The idea is to have file and dir externals behave the same
> way during
> commit, and to provide a way to recursively commit all
> externals (that are
> from the same repository and are not revision-pegged).
> 
> Who'd have guessed, there are a few corners that would be
> good to have
> others' opinions on.
> 
> (To: CMike and Bert because you two were involved in the
> original
> discussion: http://svn.haxx.se/dev/archive-2011-08/0617.shtml )
> 
> Thanks!
> ~Neels
>

Reply via email to