[Svnmerge] r22788: some discussion

Luke Call lacall186 at onemodel.org
Tue Jul 24 06:39:32 PDT 2007


Giovanni Bajo wrote:
>>> 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.

While I preferred not to have the variables say "trunk" and "branch" 
because it is not limited to that, using those words in comments helps 
the explanation because that is already a common idiom in the svn 
documentation. For that reason the comments already have *both* 
nomenclatures.  It can take a little longer to visualize what's going on 
when it only says "the copy target is the merge source, and the copy 
source is the merge target".  The most possible reader help is important 
to strive for since this part of the code has been so confusing to folks 
in the past:  multiple developers and reviewers have got it wrong or had 
to work hard to follow it, because this code is doing a non-obvious task.

Is there any specific suggested wording that would make it harder for a 
new reader to mistake what's going on, and easier to read quickly...?

Thanks again,

-Luke



More information about the Svnmerge mailing list