[PATCH 3 of 8 v8] rebase: make collapsing use explicit logic to decide on the rev to reuse
Yuya Nishihara
yuya at tcha.org
Sun Jul 3 05:24:02 EDT 2016
On Fri, 1 Jul 2016 12:30:00 +0000, Kostiantyn Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1467374993 -7200
> # Fri Jul 01 14:09:53 2016 +0200
> # Node ID 9b17aa69af8c86f2d1124a967b4289c1d2bbda1b
> # Parent e81fd23ec13aaad3fc0035994bc8a5904d23b72a
> rebase: make collapsing use explicit logic to decide on the rev to reuse
>
> This code:
>
> for rev in sortedstate:
> ...
> ...
> newnode = concludenode(repo, rev, p1, rbsrt.external,
> commitmsg=commitmsg,
> extrafn=extrafn, editor=editor,
> keepbranches=rbsrt.keepbranchesf,
> date=rbsrt.date)
>
> uses 'rev' variable in 'concludenode' function invocation. It is not
> explicitly assigned before, but its value comes as last value or 'rev' in
> a for loop, e.g. last element in a 'sortedstate'. IMO this a bad style and it
> also makes it hard to refactor the function, so it is better to explicitly
> define the value passed to 'concludenode'.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -499,10 +499,10 @@ def rebase(ui, repo, **opts):
>
> extrafn = _makeextrafn(rbsrt.extrafns)
>
> - sortedstate = sorted(rbsrt.state)
> - total = len(sortedstate)
> + rbsrt.sortedstate = sorted(rbsrt.state)
> + total = len(rbsrt.sortedstate)
> pos = 0
> - for rev in sortedstate:
> + for rev in rbsrt.sortedstate:
It's confusing that sortedstate is the list of sorted revisions.
> + revtoreuse = rbsrt.sortedstate[-1]
Instead, it can be simply written as max(self.state).
More information about the Mercurial-devel
mailing list