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

badfiles: continue execution to other files instead of stopping after a checker error #2433

Merged
merged 4 commits into from
Feb 12, 2017

Conversation

karpinski
Copy link
Contributor

Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great; thanks for splitting out a separate PR! I made a couple of suggestions, and it looks like Travis has a style suggestion as well.

Also, would you mind adding a changelog entry?

if status > 0:
ui.print_(u"{}: checker exited withs status {}"
ui.print_(u"{}: checker exited with status {}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😳 Thanks for catching this!

@@ -97,14 +102,25 @@ def check_bad(self, lib, opts, args):
ext = os.path.splitext(item.path)[1][1:].decode('utf8', 'ignore')
checker = self.get_checker(ext)
if not checker:
self._log.debug(u"no checker available for {}", ext)
self._log.error(u"no checker specified in the config for {}",
ext)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great; this is indeed a better message.

self._log.error(
u"command not found: {} when validating file: {}",
e.checker,
e.path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Travis is complaining about the indentation here. The indented lines should just be one "tab" (4 spaces) indented, and the closing parenthesis should be on a line by itself (not indented).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -30,6 +30,16 @@
import six


class CheckerCommandException(Exception):
"""Raised when running a Checker failed."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please expand the docstring just a little to explain what the fields mean (checker, path, errno, and msg)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@sampsyo sampsyo merged commit 7e6e043 into beetbox:master Feb 12, 2017
sampsyo added a commit that referenced this pull request Feb 12, 2017
badfiles: continue execution to other files instead of stopping after a checker error
sampsyo added a commit that referenced this pull request Feb 12, 2017
sampsyo added a commit that referenced this pull request Feb 12, 2017
@sampsyo
Copy link
Member

sampsyo commented Feb 12, 2017

Thank you for taking care of this so quickly! It's all merged up. ✨

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