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
add pmd xml driver #117
add pmd xml driver #117
Conversation
Method checks if the provided tool is installed. | ||
Returns: boolean False: As findbugs analyses bytecode, it would be hard to run it from outside the build framework. | ||
""" | ||
return True |
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.
I think you want return False
here.
Diff-cover is unlikely to be running findbugs, instead it will be parsing a report provided by the user.
If you intend to have diff-cover run findbugs this PR is going to need a bit more work to actually be able to run it.
for report in reports: | ||
xml_document = cElementTree.fromstring("".join(report)) | ||
files = xml_document.findall(".//file") | ||
for file in files: |
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.
Please rename the file
variable here.
File is a type in python so using it as a variable name can be confusing.
>>> file
<type 'file'>
Thanks for the PR! This is a good start! I left some notes but the bug thing is I cannot merge this unless it has some tests to cover the code being added here |
code updated |
Thanks for adding tests. I'll give this a final review and try and get it released by tomorrow |
No description provided.