[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