Patchwork [1,of,8] merge: improve debug messages for bid merge

login
register
mail settings
Submitter Mads Kiilerich
Date May 1, 2014, 11:42 p.m.
Message ID <7d8e70a9006e34e2ec12.1398987769@mk-desktop>
Download mbox | patch
Permalink /patch/4466/
State Superseded
Headers show

Comments

Mads Kiilerich - May 1, 2014, 11:42 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1398125364 -7200
#      Tue Apr 22 02:09:24 2014 +0200
# Branch stable
# Node ID 7d8e70a9006e34e2ec129f2972ce41962e35ccd9
# Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
merge: improve debug messages for bid merge
Pierre-Yves David - May 7, 2014, 9 p.m.
On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1398125364 -7200
> #      Tue Apr 22 02:09:24 2014 +0200
> # Branch stable
> # Node ID 7d8e70a9006e34e2ec129f2972ce41962e35ccd9
> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
> merge: improve debug messages for bid merge

review: improve commit for reviewer.

Specifically, include "before" and "after" output + rational for the 
change would be nice.
Mads Kiilerich - May 8, 2014, 1:29 p.m.
On 05/07/2014 11:00 PM, Pierre-Yves David wrote:
>
>
> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1398125364 -7200
>> #      Tue Apr 22 02:09:24 2014 +0200
>> # Branch stable
>> # Node ID 7d8e70a9006e34e2ec129f2972ce41962e35ccd9
>> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
>> merge: improve debug messages for bid merge
>
> review: improve commit for reviewer.

What do that sentence mean ... if it is a sentence and not some kind of 
automated test comment?

> Specifically, include "before" and "after" output + rational for the 
> change would be nice.

In principle I agree. In this case when the change is so simple and you 
know it is code I just introduced and it is made clear that it just is a 
debug message, it would be absurd and waste of time to go that much into 
details.

/Mads
Pierre-Yves David - May 8, 2014, 7:52 p.m.
On 05/08/2014 06:29 AM, Mads Kiilerich wrote:
> On 05/07/2014 11:00 PM, Pierre-Yves David wrote:
>>
>>
>> On 05/01/2014 04:42 PM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1398125364 -7200
>>> #      Tue Apr 22 02:09:24 2014 +0200
>>> # Branch stable
>>> # Node ID 7d8e70a9006e34e2ec129f2972ce41962e35ccd9
>>> # Parent  cadad384c97c7956c98f3c9b92d8cc40fa16d93b
>>> merge: improve debug messages for bid merge
>>
>> review: improve commit for reviewer.
>
> What do that sentence mean ... if it is a sentence and not some kind of
> automated test comment?

This was meant to be a joke. Not very successfully.

>> Specifically, include "before" and "after" output + rational for the
>> change would be nice.
>
> In principle I agree. In this case when the change is so simple and you
> know it is code I just introduced and it is made clear that it just is a
> debug message, it would be absurd and waste of time to go that much into
> details.

Given than multiple message seems to have changes, it can still use a 
formal clarification of what and why changed.

Its less a waste of time than two email round trips with a reviewer 
unsure about what your change is doing.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -742,8 +742,8 @@  def calculateupdates(repo, wctx, mctx, a
                                     branchmerge, force,
                                     partial, acceptremote, followcopies)
             for a in sorted(actions):
-                repo.ui.debug(' %s: %s\n' % (a[0], a[1]))
-                f = a[0]
+                f, m, args, msg = a
+                repo.ui.debug(' %s: %s -> %s\n' % (f, msg, m))
                 if f in fbids:
                     fbids[f].append(a)
                 else:
diff --git a/tests/test-merge-criss-cross.t b/tests/test-merge-criss-cross.t
--- a/tests/test-merge-criss-cross.t
+++ b/tests/test-merge-criss-cross.t
@@ -135,16 +135,16 @@  Redo merge with merge.preferancestor="*"
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 0f6b37dbe527, local: 3b08d01b0ab5+, remote: adfe50279922
-   f1: g
-   f2: m
+   f1: remote is newer -> g
+   f2: versions differ -> m
   
   calculating bids for ancestor 40663881a6dd
     searching for copies back to rev 3
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 40663881a6dd, local: 3b08d01b0ab5+, remote: adfe50279922
-   f1: m
-   f2: k
+   f1: versions differ -> m
+   f2: keep -> k
   
   auction for merging merge bids
    f1: picking 'get' action
@@ -180,16 +180,16 @@  The other way around:
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 0f6b37dbe527, local: adfe50279922+, remote: 3b08d01b0ab5
-   f1: k
-   f2: m
+   f1: keep -> k
+   f2: versions differ -> m
   
   calculating bids for ancestor 40663881a6dd
     searching for copies back to rev 3
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 40663881a6dd, local: adfe50279922+, remote: 3b08d01b0ab5
-   f1: m
-   f2: g
+   f1: versions differ -> m
+   f2: remote is newer -> g
   
   auction for merging merge bids
    f1: picking 'keep' action
@@ -246,16 +246,16 @@  Verify how the output looks and and how 
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 0f6b37dbe527, local: 3b08d01b0ab5+, remote: adfe50279922
-   f1: g
-   f2: m
+   f1: remote is newer -> g
+   f2: versions differ -> m
   
   calculating bids for ancestor 40663881a6dd
     searching for copies back to rev 3
   resolving manifests
    branchmerge: True, force: False, partial: False
    ancestor: 40663881a6dd, local: 3b08d01b0ab5+, remote: adfe50279922
-   f1: m
-   f2: k
+   f1: versions differ -> m
+   f2: keep -> k
   
   auction for merging merge bids
    f1: picking 'get' action