Patchwork [1,of,3,stable] bookmarks: remove bogus nullmerge check in updatebookmarks

login
register
mail settings
Submitter Siddharth Agarwal
Date Jan. 31, 2013, 2:16 a.m.
Message ID <63c49a4df508562b8fd9.1359598575@sid0x220>
Download mbox | patch
Permalink /patch/766/
State Superseded
Commit 26627c30735a610f59979a36885b327b25d8dbff
Headers show

Comments

Siddharth Agarwal - Jan. 31, 2013, 2:16 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1359596994 28800
# Branch stable
# Node ID 63c49a4df508562b8fd91a5da1b03b9fbadf08f8
# Parent  c795c9f87792fef98b358ae88f90c4003e9bbe4d
bookmarks: remove bogus nullmerge check in updatebookmarks

nstate[v] is a node, not an int, and the nullmerge check was done while
building nstate anyway.
Matt Mackall - Jan. 31, 2013, 5:48 a.m.
On Wed, 2013-01-30 at 18:16 -0800, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1359596994 28800
> # Branch stable
> # Node ID 63c49a4df508562b8fd91a5da1b03b9fbadf08f8
> # Parent  c795c9f87792fef98b358ae88f90c4003e9bbe4d
> bookmarks: remove bogus nullmerge check in updatebookmarks
> 
> nstate[v] is a node, not an int, and the nullmerge check was done while
> building nstate anyway.

I've queued these three for stable, thanks.

In the future, drive-by cleanups are actually not wanted in stable.
There's no upside to this sort of patch in terms of bugs fixed, user
experience improved, etc., there are only opportunities to accidentally
break things.

While this change is "obviously correct", every programmer has also had
the experience of spending hours trying to find a bug, only to discover
it was an invisible one-character typo. Thus every programmer should
also know that "obviously correct" and "correct" are not quite the same.

> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -498,9 +498,8 @@ def updatebookmarks(repo, nstate, origin
>      marks = repo._bookmarks
>      for k, v in originalbookmarks.iteritems():
>          if v in nstate:
> -            if nstate[v] > nullmerge:
> -                # update the bookmarks for revs that have moved
> -                marks[k] = nstate[v]
> +            # update the bookmarks for revs that have moved
> +            marks[k] = nstate[v]
>  
>      marks.write()
>  
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -498,9 +498,8 @@  def updatebookmarks(repo, nstate, origin
     marks = repo._bookmarks
     for k, v in originalbookmarks.iteritems():
         if v in nstate:
-            if nstate[v] > nullmerge:
-                # update the bookmarks for revs that have moved
-                marks[k] = nstate[v]
+            # update the bookmarks for revs that have moved
+            marks[k] = nstate[v]
 
     marks.write()