Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

except e: --> except Exception: #1333

Merged
merged 1 commit into from
Oct 14, 2018
Merged

except e: --> except Exception: #1333

merged 1 commit into from
Oct 14, 2018

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Nov 14, 2017

In the current code e is both undefined and unused so this code will raise an NameError exception which masks the original exception.

An alternative solution would be: except Exception as e: but that still creates an unused variable.

Checklist
  • npm install && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

@cclauss
Copy link
Contributor Author

cclauss commented Oct 14, 2018

@refack Your review please.

flake8 testing of https://github.com/nodejs/node-gyp on Python 2.7.14

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./gyp/pylib/gyp/mac_tool.py:129:12: F821 undefined name 'e'
    except e:
           ^
./gyp/pylib/gyp/xcode_emulation.py:846:9: F821 undefined name 'cflags'
        cflags.append('-F' + platform_root + '/Developer/Library/Frameworks/')
        ^
./gyp/pylib/gyp/generator/make.py:1642:50: F821 undefined name 'target'
      print("WARNING: no output for", self.type, target)
                                                 ^
3     F821 undefined name 'target'
3

@refack refack self-assigned this Oct 14, 2018
PR-URL: #1333
Reviewed-By: Refael Ackermann <[email protected]>
@refack
Copy link
Contributor

refack commented Oct 14, 2018

@refack refack merged commit ae0be87 into nodejs:master Oct 14, 2018
@cclauss cclauss deleted the patch-2 branch October 14, 2018 21:41
rvagg pushed a commit that referenced this pull request Apr 24, 2019
PR-URL: #1333
Reviewed-By: Refael Ackermann <[email protected]>
@rvagg rvagg mentioned this pull request Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants