Skip to content

Commit

Permalink
Merge pull request #51 from stdweird/fix_headers
Browse files Browse the repository at this point in the history
Fix fix headers exception handling
  • Loading branch information
Jens Timmerman committed Aug 30, 2016
2 parents f73f56d + 429c20e commit d584b06
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 19 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -7,6 +7,7 @@ setup.cfg
MANIFEST

# Packages
.eggs
*.egg
*.egg-info
dist
Expand Down
47 changes: 32 additions & 15 deletions README.md
Expand Up @@ -118,14 +118,14 @@ try:
# something
except:
```
This is bad, because this except will also catch sys.exit() or Keyboardinterupts, something you
typically do not want, if you catch these the program will be in a weird state and then continue on,
This is bad, because this except will also catch sys.exit() or Keyboardinterupts, something you
typically do not want, if you catch these the program will be in a weird state and then continue on,
whilst the person who just pressed ctrl+c is wondering what is going on and why it is not stopping.

so at the very least make this
except Exception (which doesn't catch sys.exit and KeyboardInterupt)
and it would be appreciated if you could actually figure out what exceptions to expect and only catch those
and let your program crash if something you did not intend happens
and let your program crash if something you did not intend happens
because it helps developers catch weird errors on their side better.

if you do something like
Expand All @@ -136,7 +136,7 @@ except Exception:
pass # if somestring is not an integer, we didn't need to write anyway, but otherwise we do
```
because you know sometimes this string does not contain an integer, so the int() call can fail
you should really only catch ValueError, because this will also fail when your disk is full, or you don't have permissions
you should really only catch ValueError, because this will also fail when your disk is full, or you don't have permissions
or xxx other reasons, and the important data will not be written out and nobody will notice anything!


Expand All @@ -151,33 +151,33 @@ Also helps code review, not in reads better, like english.
arguments-differ
-----------------

this will give you errors if you override a function of a superclass but don't use the same amount of arguments,
this will give you errors if you override a function of a superclass but don't use the same amount of arguments,
using less will surely give you errors, so the linter catches this for you now

unused-argument
-----------------
if you have a function definition witch accepts an argument that is never used in the function body this will now give an error.
if you have a function definition witch accepts an argument that is never used in the function body this will now give an error.
clean up your function definition, or fix the error where you actually do need to take this argument into account

unused-variable
----------------
defining a variable and then not using it anymore smells bad, why did you do that?
sometimes you do things like
```python
out, exit_code = run_command(something)
out, exit_code = run_command(something)
```
but you are not interested in the out, only in the exit code,
in this case, write
but you are not interested in the out, only in the exit code,
in this case, write
```python
_, exit_code = run_command(something)
_, exit_code = run_command(something)
```

using _ as a variable name lets pylint and other readers know you do not intend to use that output in the first place.


reimported
-------------
when you re import a name somewhere else,
when you re import a name somewhere else,
usually this is just an import to much, or 2 imports with the same name, pay attention.
```python
import six
Expand All @@ -200,16 +200,31 @@ class VscGroup(object):
pass
```

=> do you need the import? use import as
=> do you need the import? use import as
did you mean to use the same name? ...

unpacking-in-except / redefine-in-handler
-----------------------------------------

Multiple exception have to be grouped in a tuple like

```python
...
except (ExceptionOne, ExceptionTwo) ...
...
```

(espcially when used like `except A, B:` which should be `except (A, B):`.

turning off these errors
-------------------------


if in any of these cases you think: yes, I really needed to do this, I'm monkeypatching things, I'm adding extra functionality that does indeed have an extra(default) paramenter, etc, etc
If in any of these cases you think: yes, I really needed to do this,
I'm monkeypatching things, I'm adding extra functionality that does indeed have an extra(default) paramenter, etc, etc
you can let pylint know to ignore this error in this one specific block of code
by adding e.g.the comment
by adding e.g. the comment `# pylint: disable=<name or numeric id of pylint code>`

```python
class Something(object):
def dosomething(self, some, thing):
Expand All @@ -218,5 +233,7 @@ class Something(object):
class MyFancyThing(SomeThing):
# pylint: disable=arguments-differ
def dosomething(self, some, thing, fancy=None):
# do soemthing fancy
# do something fancy
```

Full list with all codes is available at http://pylint-messages.wikidot.com/all-codes
1 change: 1 addition & 0 deletions lib/vsc/install/commontest.py
Expand Up @@ -103,6 +103,7 @@ class CommonTest(TestCase):
'F811', # redefinition of unused name
'unused-import',
'syntax-error',
'unpacking-in-except', 'redefine-in-handler', # except A, B -> except (A, B)
#'protected-access',
#'logging-not-lazy',
]
Expand Down
2 changes: 1 addition & 1 deletion lib/vsc/install/headers.py
Expand Up @@ -314,7 +314,7 @@ def check_header(filename, script=False, write=False):
args = sys.argv[1:]
try:
is_script = int(args[-1]) == 1
except IndexError, ValueError:
except (IndexError, ValueError):
is_script = False

if is_script:
Expand Down
14 changes: 11 additions & 3 deletions lib/vsc/install/shared_setup.py
Expand Up @@ -63,8 +63,10 @@
except ImportError:
have_xmlrunner = False


# Test that these are matched by a .gitignore pattern
GITIGNORE_PATTERNS = ['.pyc', '.pyo', '~']
# .gitnore needs to contain these exactly
GITIGNORE_EXACT_PATTERNS = ['.eggs']

# private class variables to communicate
# between VscScanningLoader and VscTestCommand
Expand Down Expand Up @@ -145,7 +147,7 @@ def _log(self, level, msg, args):

RELOAD_VSC_MODS = False

VERSION = '0.10.12'
VERSION = '0.10.13'

log.info('This is (based on) vsc.install.shared_setup %s' % VERSION)

Expand Down Expand Up @@ -373,12 +375,18 @@ def rel_gitignore(self, paths, base_dir=None):
# primitive gitignore
gitignore = os.path.join(base_dir, '.gitignore')
if os.path.isfile(gitignore):
patterns = [l.strip().replace('*', '.*') for l in open(gitignore).readlines() if l.startswith('*')]
all_patterns = [l for l in [l.strip() for l in open(gitignore).readlines()] if l and not l.startswith('#')]

patterns = [l.replace('*', '.*') for l in all_patterns if l.startswith('*')]
reg = re.compile('^('+'|'.join(patterns)+')$')

# check if we at least filter out .pyc files, since we're in a python project
if not all([reg.search(text) for text in ['bla%s' % pattern for pattern in GITIGNORE_PATTERNS]]):
raise Exception("%s/.gitignore does not contain these patterns: %s " % (base_dir, GITIGNORE_PATTERNS))

if not all([l in all_patterns for l in GITIGNORE_EXACT_PATTERNS]):
raise Exception("%s/.gitignore does not contain all following patterns: %s " % (base_dir, GITIGNORE_EXACT_PATTERNS))

res = [f for f in res if not reg.search(f)]

elif os.path.isdir(os.path.join(base_dir, '.git')):
Expand Down

0 comments on commit d584b06

Please sign in to comment.