-
Notifications
You must be signed in to change notification settings - Fork 581
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
JavaPMDBear: Use dependency_management #1667
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,22 +17,14 @@ class JavaPMDBear: | |
""" | ||
|
||
LANGUAGES = {'Java'} | ||
REQUIREMENTS = {MavenRequirement('net.sourceforge.pmd:pmd', '5.6.0'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our CI test is using v 5.4.1 https://github.com/coala/coala-bears/blob/master/.ci/deps.sh#L163 But if you already check that it's not breaking then OK (and maybe we need to update the CI script) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion @adhikasp will surely fix this. But a lot of work needs to be done first. 😃 |
||
DistributionRequirement('bash')} | ||
AUTHORS = {'The coala developers'} | ||
AUTHORS_EMAILS = {'[email protected]'} | ||
LICENSE = 'AGPL-3.0' | ||
CAN_DETECT = {'Code Simplification', 'Unreachable Code', 'Smell', | ||
'Duplication'} | ||
|
||
@classmethod | ||
def check_prerequisites(cls): # pragma: no cover | ||
if which('bash') is None: | ||
return 'bash is not installed.' | ||
elif which('pmd') is None and which('run.sh') is None: | ||
return ('PMD is missing. Make sure to install it from ' | ||
'<https://pmd.github.io/>') | ||
else: | ||
return True | ||
|
||
@staticmethod | ||
@deprecate_settings(allow_unnecessary_code=('check_unnecessary', negate), | ||
allow_unused_code=('check_unused', negate)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be a breaking change.
MavenRequirement
needs to be an optional way to obtainpmd
.And I am pretty sure that the maven
pmd
doesnt install apmd
executable.i.e. you need to test that old and new work, which is why this is listed as medium difficulty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry my bad just overlooked this and thought it would merely be adding the requirement line. Will fix this shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to import
MavenRequirement
.