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 >