Patchwork [3,of,3] rebase: shift all states constant by one

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 2, 2014, 8:32 p.m.
Message ID <4dbdc7f27f3a78ae719d.1417552344@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6960/
State Superseded
Headers show

Comments

Pierre-Yves David - Dec. 2, 2014, 8:32 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1417551939 28800
#      Tue Dec 02 12:25:39 2014 -0800
# Node ID 4dbdc7f27f3a78ae719d5f9826c6245c4b48734e
# Parent  627b344d81f64444216dc0ac7cfc33d63d7fcdff
rebase: shift all states constant by one

This make 'revtoto' not overlapping with nullrev anymore. As a side effect it
a rebase started with 3.2 and resumed with 3.3 will get confused. So I'm not
sure this change a is good idea. The fact is pass tests "proves" all relevant place
are using the right variable.
Mads Kiilerich - Dec. 3, 2014, 1:05 a.m.
On 12/02/2014 09:32 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417551939 28800
> #      Tue Dec 02 12:25:39 2014 -0800
> # Node ID 4dbdc7f27f3a78ae719d5f9826c6245c4b48734e
> # Parent  627b344d81f64444216dc0ac7cfc33d63d7fcdff
> rebase: shift all states constant by one
>
> This make 'revtoto' not overlapping with nullrev anymore. As a side effect it

revtodo

> a rebase started with 3.2 and resumed with 3.3 will get confused.

Starting with 3.3 and continuing with older versions will also misbehave.

> So I'm not sure this change a is good idea.

Agreed, it shouldn't be applied, but it has value as proof.

> The fact is pass tests "proves" all relevant place are using the right variable.

Agreed. Nice catch with the "negative state update back" fix that made 
it less dependent of revtodo==nullrev.

IMO, if you want to make revtodo independent from nullrev and just have 
it as an arbitrary enum without any secret traps or hidden dependencies, 
then the dependency on the ordering of the constants should be removed too.

/Mads

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -21,13 +21,13 @@  from mercurial.commands import templateo
 from mercurial.node import nullrev
 from mercurial.lock import release
 from mercurial.i18n import _
 import os, errno
 
-revtodo = -1
-nullmerge = -2
-revignored = -3
+revtodo = -2
+nullmerge = -3
+revignored = -4
 
 cmdtable = {}
 command = cmdutil.command(cmdtable)
 testedwith = 'internal'