Allow parallel lint and remove broken "verbose" mode #36
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ldoktor
force-pushed
the
parallel
branch
2 times, most recently
from
4900b12
to
16b68c7
Compare
Pylint supports parallel execution, let's allow it in inspekt as well. One draw-back of this is that when parallel pylint is enabled, it does not reports passed tests as usual, therefor there is no simple way of adding our "Lint: %(path)s %(status)" debug messages. It is not impossible, but not feasible, therefor this version prints the list of results after the execution. Also instead of filename it prints module name (for the same reason). Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
The "--verbose" argument is already consumed by cliff's App() and is used to set logging levels. It is not passed to sub-commands and therefor the removed code in lint did not work. Using "--verbose" still changes the logging levels, so all the "checker.log_status()" messages are only printed when "--verbose" is enabled. This patch removes the original "verbose" mode for "pylint" because it does not actually work every since we moved to cliff. Since no one complained about it I think it's better to remove it and keep the current behavior which is to run normal pylint and log the "checker.log_status()" messages in "verbose" mode. Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
The newly introduced W605 informs about places where non-existing escape is used, which used to be automatically escaped but is about to become syntax error. Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
clebergnu
approved these changes
Apr 18, 2018
inspektor/lint.py
Outdated
| @@ -54,9 +54,13 @@ def __init__(self, args, logger=logging.getLogger('')): | |||
| self.log = logger | |||
| self.args = args | |||
| self.verbose = args.verbose | |||
| if hasattr(args, 'parallel'): | |||
There was a problem hiding this comment.
I was really intrigued on why this was necessary: basically the default should set this always. Then I remember that sometimes lint is called by checkall, so yes, LGTM.
There was a problem hiding this comment.
exactly (and Travis actually reminded me this on this very PR)
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit adds "parallel" execution of "inspekt lint" and the second one removes the "--verbose" mode from all sub-commands, because it is not really passed as an argument. Partially it works in the sense the loggers are set by the main app to DEBUG vs. INFO leading to some messages being printed, but the different setting for lint does not work. As this issue is with us for quite a while I choose to remove it and keep only to already working DEBUG vs. INFO part as it might be useful to people.
Note: It'd be nice to release new inspektor with this commit to speedup the Travis executions.