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

Make the thrift linter use the standard linter mixin. #5394

Merged
merged 4 commits into from Jan 27, 2018

Conversation

Projects
None yet
3 participants
@benjyw
Copy link
Contributor

benjyw commented Jan 26, 2018

Deprecates its old goal (thrift-linter) in favor of the
standard lint goal.

Make the thrift linter use the standard linter mixin.
Deprecates its old goal (thrift-linter) in favor of the
standard lint goal.

@benjyw benjyw requested a review from baroquebobcat Jan 26, 2018

@@ -21,7 +23,7 @@ class ThriftLintError(Exception):
"""Raised on a lint failure."""


class ThriftLinter(NailgunTask):
class ThriftLinterBase(NailgunTask):
"""Print linter warnings for thrift files."""

This comment has been minimized.

@baroquebobcat

baroquebobcat Jan 26, 2018

Contributor

Should we push this docstring down to the subclasses? We currently don't look at super classes docstrings when determining the task help text, so both of the sub tasks have no description.

This comment has been minimized.

@benjyw

benjyw Jan 26, 2018

Contributor

Good idea. Done.

Move docstring to concrete task subclass.
So help text is correctly generated.

@benjyw benjyw merged commit cedaa03 into pantsbuild:master Jan 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@benjyw benjyw deleted the benjyw:thrift_lint_fix branch Jan 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment