Patchwork histedit: remove extra histedit constructor call

login
register
mail settings
Submitter Durham Goode
Date March 11, 2015, 2:20 p.m.
Message ID <c00fe123cfeb0a8935a3.1426083657@dev2000.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8004/
State Accepted
Headers show

Comments

Durham Goode - March 11, 2015, 2:20 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1426083595 25200
#      Wed Mar 11 07:19:55 2015 -0700
# Node ID c00fe123cfeb0a8935a3b636da31f23eee6c67ba
# Parent  913347bcd59c33110f029d83203b3b4c2ecd91a8
histedit: remove extra histedit constructor call

In a previous commit we removed the extra histedit object instance being
constructed in --continue and --abort. The new --edit-todo missed this fix
though (which means the state object it produces doesn't have the locks on it).
It's not breaking anything now, but let's go ahead and clean that up before we
forget.
Ryan McElroy - March 11, 2015, 3:44 p.m.
On 3/11/2015 7:20 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1426083595 25200
> #      Wed Mar 11 07:19:55 2015 -0700
> # Node ID c00fe123cfeb0a8935a3b636da31f23eee6c67ba
> # Parent  913347bcd59c33110f029d83203b3b4c2ecd91a8
> histedit: remove extra histedit constructor call
>
> In a previous commit we removed the extra histedit object instance being
> constructed in --continue and --abort. The new --edit-todo missed this fix
> though (which means the state object it produces doesn't have the locks on it).
> It's not breaking anything now, but let's go ahead and clean that up before we
> forget.
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -616,7 +616,6 @@ def _histedit(ui, repo, state, *freeargs
>           state.read()
>           state = bootstrapcontinue(ui, state, opts)
>       elif goal == 'edit-plan':
> -        state = histeditstate(repo)
>           state.read()
>           if not rules:
>               comment = editcomment % (state.parentctx, node.short(state.topmost))

Good catch. Looks good to me.
Augie Fackler - March 11, 2015, 3:50 p.m.
LGTM, queued.

On Mar 11, 2015, at 10:20 AM, Durham Goode <durham@fb.com> wrote:

> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1426083595 25200
> #      Wed Mar 11 07:19:55 2015 -0700
> # Node ID c00fe123cfeb0a8935a3b636da31f23eee6c67ba
> # Parent  913347bcd59c33110f029d83203b3b4c2ecd91a8
> histedit: remove extra histedit constructor call
> 
> In a previous commit we removed the extra histedit object instance being
> constructed in --continue and --abort. The new --edit-todo missed this fix
> though (which means the state object it produces doesn't have the locks on it).
> It's not breaking anything now, but let's go ahead and clean that up before we
> forget.
> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -616,7 +616,6 @@ def _histedit(ui, repo, state, *freeargs
>         state.read()
>         state = bootstrapcontinue(ui, state, opts)
>     elif goal == 'edit-plan':
> -        state = histeditstate(repo)
>         state.read()
>         if not rules:
>             comment = editcomment % (state.parentctx, node.short(state.topmost))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - March 30, 2015, 12:20 a.m.
On Wed, Mar 11, 2015 at 08:44:09AM -0700, Ryan McElroy wrote:
> On 3/11/2015 7:20 AM, Durham Goode wrote:
> ># HG changeset patch
> ># User Durham Goode <durham@fb.com>
> ># Date 1426083595 25200
> >#      Wed Mar 11 07:19:55 2015 -0700
> ># Node ID c00fe123cfeb0a8935a3b636da31f23eee6c67ba
> ># Parent  913347bcd59c33110f029d83203b3b4c2ecd91a8
> >histedit: remove extra histedit constructor call
> >
> >In a previous commit we removed the extra histedit object instance being
> >constructed in --continue and --abort. The new --edit-todo missed this fix
> >though (which means the state object it produces doesn't have the locks on it).
> >It's not breaking anything now, but let's go ahead and clean that up before we
> >forget.
> >
> >diff --git a/hgext/histedit.py b/hgext/histedit.py
> >--- a/hgext/histedit.py
> >+++ b/hgext/histedit.py
> >@@ -616,7 +616,6 @@ def _histedit(ui, repo, state, *freeargs
> >          state.read()
> >          state = bootstrapcontinue(ui, state, opts)
> >      elif goal == 'edit-plan':
> >-        state = histeditstate(repo)
> >          state.read()
> >          if not rules:
> >              comment = editcomment % (state.parentctx, node.short(state.topmost))
>
> Good catch. Looks good to me.

Queued as 365fdbc54f1f.

(I'm testing a new feature on the patchbot. I apologize for this
slight thread necromancy.)

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -616,7 +616,6 @@  def _histedit(ui, repo, state, *freeargs
         state.read()
         state = bootstrapcontinue(ui, state, opts)
     elif goal == 'edit-plan':
-        state = histeditstate(repo)
         state.read()
         if not rules:
             comment = editcomment % (state.parentctx, node.short(state.topmost))