5:09 p.m.
The first was inspired by a class of pylint errors, the C (Convention) ones
that are disabled in our tests. I happened to run pylint-gui for fun and this
one popped out at me by pure luck.
However, if the C error were undisabled there would be about
3000 new errors, mostly quite pointless, about too-long-lines and missing-
docstrings.
The one that made me notice bug in patch (1) was the superfluous-parens
error. There are 55 other instances of just this, but they all appear benign.
Fixing them wouldn't be hard, but it would be a busy commit with no real
useful affect.
So I'm leaving the C errors disabled.
mulhern (2):
Match whole string, not substring.
Remove redundant import.
blivet/arch.py | 2 +-
tests/size_test.py | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
--
1.9.3
5:09 p.m.
New subject: [blivet:master 1/2] Match whole string, not substring.
The expression on the right of the 'in' was being interpreted as a str
with some pointless parens around it. Now it is interpreted as a tuple.
Signed-off-by: mulhern <amulhern(a)redhat.com>
---
blivet/arch.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/arch.py b/blivet/arch.py
index 810ec1c..cd93d86 100644
--- a/blivet/arch.py
+++ b/blivet/arch.py
@@ -285,7 +285,7 @@ def isPPC(bits=None):
if arch in ('ppc', 'ppc64', 'ppc64le'):
return True
elif bits == 32:
- if arch in ('ppc'):
+ if arch in ('ppc',):
return True
elif bits == 64:
if arch in ('ppc64', 'ppc64le'):
--
1.9.3
1:21 a.m.
New subject: [blivet:master 1/2] Match whole string, not substring.
On Tue, 2014-11-18 at 18:09 -0500, mulhern wrote:
The expression on the right of the 'in' was being interpreted
as a str
with some pointless parens around it. Now it is interpreted as a tuple.
Signed-off-by: mulhern <amulhern(a)redhat.com>
---
blivet/arch.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/blivet/arch.py b/blivet/arch.py
index 810ec1c..cd93d86 100644
--- a/blivet/arch.py
+++ b/blivet/arch.py
@@ -285,7 +285,7 @@ def isPPC(bits=None):
if arch in ('ppc', 'ppc64', 'ppc64le'):
return True
elif bits == 32:
- if arch in ('ppc'):
+ if arch in ('ppc',):
Wouldn't it be better to use 'if arch
== 'ppc':' here instead? I know
the usage of 'in' is a common pattern in that code, but it just looks
weird to me.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
6:25 a.m.
New subject: [blivet:master 1/2] Match whole string, not substring.
----- Original Message -----
From: "Vratislav Podzimek" <vpodzime(a)redhat.com>
To: "anaconda patch review" <anaconda-patches(a)lists.fedorahosted.org>
Sent: Wednesday, November 19, 2014 2:21:45 AM
Subject: Re: [blivet:master 1/2] Match whole string, not substring.
On Tue, 2014-11-18 at 18:09 -0500, mulhern wrote:
> The expression on the right of the 'in' was being interpreted as a str
> with some pointless parens around it. Now it is interpreted as a tuple.
>
> Signed-off-by: mulhern <amulhern(a)redhat.com>
> ---
> blivet/arch.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/blivet/arch.py b/blivet/arch.py
> index 810ec1c..cd93d86 100644
> --- a/blivet/arch.py
> +++ b/blivet/arch.py
> @@ -285,7 +285,7 @@ def isPPC(bits=None):
> if arch in ('ppc', 'ppc64', 'ppc64le'):
> return True
> elif bits == 32:
> - if arch in ('ppc'):
> + if arch in ('ppc',):
Wouldn't it be better to use 'if arch == 'ppc':' here instead? I
know
the usage of 'in' is a common pattern in that code, but it just looks
weird to me.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Changed!
- mulhern
5:09 p.m.
New subject: [blivet:master 2/2] Remove redundant import.
Signed-off-by: mulhern <amulhern(a)redhat.com>
---
tests/size_test.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/size_test.py b/tests/size_test.py
index 959d8ac..f4acb72 100644
--- a/tests/size_test.py
+++ b/tests/size_test.py
@@ -307,7 +307,6 @@ class UtilityMethodsTestCase(unittest.TestCase):
self.assertEqual(size._lowerASCII(b"B"), b"b")
def testArithmetic(self):
- from decimal import Decimal
s = Size("2GiB")
# Make sure arithmatic operations with Size always result in the expected type
--
1.9.3
1:22 a.m.
On Tue, 2014-11-18 at 18:09 -0500, mulhern wrote:
The first was inspired by a class of pylint errors, the C
(Convention) ones
that are disabled in our tests. I happened to run pylint-gui for fun and this
one popped out at me by pure luck.
However, if the C error were undisabled there would be about
3000 new errors, mostly quite pointless, about too-long-lines and missing-
docstrings.
The one that made me notice bug in patch (1) was the superfluous-parens
error. There are 55 other instances of just this, but they all appear benign.
Fixing them wouldn't be hard, but it would be a busy commit with no real
useful affect.
So I'm leaving the C errors disabled.
A good strategy, I'd say. :) Other
than on suggestion these both look
good to me.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
3483
days inactive
3484
days old
anaconda-patches@lists.fedorahosted.org
5 comments
2 participants
participants (2)
-
Anne Mulhern -
Vratislav Podzimek