Patchwork [1,of,4,stable] shelve: remove unused variable assignment

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 28, 2013, 10 p.m.
Message ID <a45cf16389bcf2fb64dd.1382997652@mk-desktop>
Download mbox | patch
Permalink /patch/2829/
State Superseded
Commit 065e6f1c92593af8ca85708cee083f2999c5eb1a
Headers show

Comments

Mads Kiilerich - Oct. 28, 2013, 10 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1382995940 -3600
#      Mon Oct 28 22:32:20 2013 +0100
# Branch stable
# Node ID a45cf16389bcf2fb64dd95047bf0357e8313c83c
# Parent  1d7a36ff2615e6e12ce65681db4e07622e7b60d1
shelve: remove unused variable assignment

Fix test-check-pyflakes.t error after 1d7a36ff2615.
Augie Fackler - Oct. 31, 2013, 12:42 p.m.
On Mon, Oct 28, 2013 at 11:00:52PM +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1382995940 -3600
> #      Mon Oct 28 22:32:20 2013 +0100
> # Branch stable
> # Node ID a45cf16389bcf2fb64dd95047bf0357e8313c83c
> # Parent  1d7a36ff2615e6e12ce65681db4e07622e7b60d1
> shelve: remove unused variable assignment

Dropping this one, it breaks shelve, at least with foozy's patches
applied.

>
> Fix test-check-pyflakes.t error after 1d7a36ff2615.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -532,8 +532,6 @@
>      else:
>          basename = shelved[0]
>
> -    shelvedfiles = readshelvedfiles(repo, basename)
> -
>      wlock = lock = tr = None
>      try:
>          lock = repo.lock()
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Katsunori FUJIWARA - Oct. 31, 2013, 3:22 p.m.
At Thu, 31 Oct 2013 08:42:01 -0400,
Augie Fackler wrote:
> 
> On Mon, Oct 28, 2013 at 11:00:52PM +0100, Mads Kiilerich wrote:
> > # HG changeset patch
> > # User Mads Kiilerich <madski@unity3d.com>
> > # Date 1382995940 -3600
> > #      Mon Oct 28 22:32:20 2013 +0100
> > # Branch stable
> > # Node ID a45cf16389bcf2fb64dd95047bf0357e8313c83c
> > # Parent  1d7a36ff2615e6e12ce65681db4e07622e7b60d1
> > shelve: remove unused variable assignment
> 
> Dropping this one, it breaks shelve, at least with foozy's patches
> applied.

This patch breaks shelve, even if my patches are not applied.

"readshelvedfiles()" invocation here is requierd to ensure that
shelved changes named as specified exist after that line.

So, checking like below should be done, if "readshelvedfiles()"
invocation is removed.

    diff -r 15ec2bc8fa65 hgext/shelve.py
    --- a/hgext/shelve.py   Thu Oct 31 23:47:42 2013 +0900
    +++ b/hgext/shelve.py   Fri Nov 01 00:08:19 2013 +0900
    @@ -523,7 +523,8 @@
         else:
             basename = shelved[0]
    
    -    shelvedfiles = readshelvedfiles(repo, basename)
    +    if not shelvedfile(repo, basename, 'files').exists():
    +        raise util.Abort(_("shelved change '%s' not found") % basename)
    
         wlock = lock = tr = None
         try:

BTW, definition of "readshelvedfiles()" itself also seems removable,
because it is only referred by the removed line.


> >
> > Fix test-check-pyflakes.t error after 1d7a36ff2615.
> >
> > diff --git a/hgext/shelve.py b/hgext/shelve.py
> > --- a/hgext/shelve.py
> > +++ b/hgext/shelve.py
> > @@ -532,8 +532,6 @@
> >      else:
> >          basename = shelved[0]
> >
> > -    shelvedfiles = readshelvedfiles(repo, basename)
> > -
> >      wlock = lock = tr = None
> >      try:
> >          lock = repo.lock()
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Mads Kiilerich - Oct. 31, 2013, 3:48 p.m.
On 10/31/2013 04:22 PM, FUJIWARA Katsunori wrote:
> At Thu, 31 Oct 2013 08:42:01 -0400,
> Augie Fackler wrote:
>> On Mon, Oct 28, 2013 at 11:00:52PM +0100, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1382995940 -3600
>>> #      Mon Oct 28 22:32:20 2013 +0100
>>> # Branch stable
>>> # Node ID a45cf16389bcf2fb64dd95047bf0357e8313c83c
>>> # Parent  1d7a36ff2615e6e12ce65681db4e07622e7b60d1
>>> shelve: remove unused variable assignment
>> Dropping this one, it breaks shelve, at least with foozy's patches
>> applied.
> This patch breaks shelve, even if my patches are not applied.
>
> "readshelvedfiles()" invocation here is requierd to ensure that
> shelved changes named as specified exist after that line.

So you say that this pretty pure function with a return value that is 
assigned to an unused variable is necessary for the not-so-pure side 
effect that the function can raise exceptions. Ok.

Your patch seems good - it fixes both the need for a comment and the 
pyflakes failure.

/Mads

Patch

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -532,8 +532,6 @@ 
     else:
         basename = shelved[0]
 
-    shelvedfiles = readshelvedfiles(repo, basename)
-
     wlock = lock = tr = None
     try:
         lock = repo.lock()