Hi,

When going through the code for 1.14.1, I looked at r1880192. Is it
intentional to have both comments?

When I'm reading the comment and trying to understand the code I'm half
expecting to have two different (possibly nested) loops.

   /* Iterate over each path with explicit mergeinfo added by the merge. */
+  /* Iterate over the paths in a parent-to-child order so that inherited
+   * mergeinfo is propagated consistently from each parent path to its
+   * children. (Issue #4862) */

It would make it easier to understand (at least for me) if it was a single
comment. (Unless I'm missing something in how the code is supposed to work
in which case I'm be happy to learn.)

Index: merge.c
===================================================================
--- merge.c     (revision 1885615)
+++ merge.c     (working copy)
@@ -7921,10 +7921,9 @@
   if (!merge_b->paths_with_new_mergeinfo || merge_b->dry_run)
     return SVN_NO_ERROR;

-  /* Iterate over each path with explicit mergeinfo added by the merge. */
-  /* Iterate over the paths in a parent-to-child order so that inherited
-   * mergeinfo is propagated consistently from each parent path to its
-   * children. (Issue #4862) */
+  /* Iterate over the paths with explicit mergeinfo added by the merge
+   * in a parent-to-child order so that inherited mergeinfo is propagated
+   * consistently from each parent path to its children. (Issue #4862) */
   a = svn_sort__hash(merge_b->paths_with_new_mergeinfo,
                      svn_sort_compare_items_as_paths, pool);
   iterpool = svn_pool_create(pool);

(Even though I stumbled on this up while reviewing 1.14.1, it is not
something that should be backported).

Kind regards,
Daniel

Reply via email to