| 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
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.
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
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