Message ID | 20220112134635.177877-18-jholdsworth@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | git-p4: Various code tidy-ups | expand |
On Wed, Jan 12, 2022 at 8:47 AM Joel Holdsworth <jholdsworth@nvidia.com> wrote: > PEP8 recommends that comparisons with singletons such as None should be > done with "is" and "is not", and never equality operators. > > This guideline is described here: > https://www.python.org/dev/peps/pep-0008/#programming-recommendations > > Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com> > --- > diff --git a/git-p4.py b/git-p4.py > @@ -930,7 +930,7 @@ def p4Where(depotPath): > if data[:space] == depotPath: > output = entry > break > - if output == None: > + if not output: > return "" This changes semantics, doesn't it? It will now return "" for None, False, and "". Is this change intentional? To match the intent of the original, I would have expected it to be written: if output is None: return "" but, having glanced at the larger context, I suppose the new semantics may be okay(?). > def parseRevision(ref): > @@ -4497,7 +4497,7 @@ def main(): > if cmd.needsGit: > - if cmd.gitdir == None: > + if not cmd.gitdir: > cmd.gitdir = os.path.abspath(".git") Same question/observation as above. In order to save reviewers some time, perhaps the commit message could say something about why the semantic change is safe and/or desirable.
diff --git a/git-p4.py b/git-p4.py index 0159e35adc..2a3409ebd2 100755 --- a/git-p4.py +++ b/git-p4.py @@ -930,7 +930,7 @@ def p4Where(depotPath): if data[:space] == depotPath: output = entry break - if output == None: + if not output: return "" if output["code"] == "error": return "" @@ -952,7 +952,7 @@ def currentGitBranch(): def isValidGitDir(path): - return git_dir(path) != None + return git_dir(path) is not None def parseRevision(ref): @@ -4497,7 +4497,7 @@ def main(): global verbose verbose = cmd.verbose if cmd.needsGit: - if cmd.gitdir == None: + if not cmd.gitdir: cmd.gitdir = os.path.abspath(".git") if not isValidGitDir(cmd.gitdir): # "rev-parse --git-dir" without arguments will try $PWD/.git
PEP8 recommends that comparisons with singletons such as None should be done with "is" and "is not", and never equality operators. This guideline is described here: https://www.python.org/dev/peps/pep-0008/#programming-recommendations Signed-off-by: Joel Holdsworth <jholdsworth@nvidia.com> --- git-p4.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)