[Svnmerge] r22788: some discussion

[Giovanni Bajo]

On 7/23/2007 8:49 PM, Luke Call wrote:

>> OK but comments shouldn't be in docstring format. In python you use "#"
>> for commands, and strings for docstrings. The two have different
>> meanings and usages, even though they have the same net effect on the
>> code (they are ignored).
> 
> That makes word wrap a nuisance, but I've changed it.

That pretty much depends on the editor you're using. Komodo, for 
instance, offers a word-wrapping facility which correctly respects comments.

>> Luke, many thanks for your "passion" into this bug. I think we're almost
>> there. I think next steps are:
> 
> My goal is to simplify usage for our developers, and I think it will 
> help others also.  There are now two common (for us) scenarios (where 
> either branch is a copy of the other) where the default behavior 
> (without a revision list parameter) works as expected instead of 
> requiring them to review logs and calculate rev numbers.  Also to make 
> it easier for me to integrate future changes into our diverged 
> svnmerge.py, if at least part of the divergence will have been committed 
> upstream.  If others benefit, that is a most happy thought as well.
> 
>> 1) Patch #1 can be committed (Dustin, can you can take care of that?)
> Thanks!!!  (And to Dustin for checking it in.)
> 
>> 2) Patch #2 should be revised as I suggested, by just returning None on
>> optional attributes when they're not available.
> 
> Done; uploaded here: 
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/675/addNullreturnsToPulldomUsage.patch 

OK, approved. FYI, a shortest way (which doesn't introduce a new 
function) is to repeat this simple pattern:

     def action(self):
         return getAttributeOrNone(self._node, "action") or None

>> 3) A separate patch for renaming branch->target (and doing only that) is
>> pre-approved and can be committed as soon as it is submitted.
> 
> done; available here: 
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/676/renameParametersToActionInit.patch 

OK, approved.

>> 4) Patch #3 needs only minor nits (as reviewed here). I want to
>> re-review it when it's finished, but I don't foresee any specific
>> problem. Also please submit this one in context diff format (if
>> possible) as the unified format isn't clear in this case.
> 
> Done; now available here (context format): 
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/677/furtherInitLogicFixesAsBegunByr22788-take3-unifiedFormat.patch 
> 
> 
> ..and here (unified format): 
> http://subversion.tigris.org/nonav/issues/showattachment.cgi/678/furtherInitLogicFixesAsBegunByr22788-take3-contextFormat.patch 

OK, approved, with one minor correction: the comments in action_init() 
still speaks of "branch" and "trunk". You should updated them to speak 
about "source" and "target", otherwise they don't match anymore with the 
code.

Thanks again for working on this!
-- 
Giovanni Bajo