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

DS-3712: Add Error Prone checks to catch common Java code mistakes #1861

Merged
merged 5 commits into from Oct 11, 2017

Conversation

Projects
None yet
3 participants
@tdonohue
Copy link
Member

commented Oct 5, 2017

This PR adds and enables Google's ErrorProne checks: https://github.com/google/error-prone

https://jira.duraspace.org/browse/DS-3712

It also fixes bugs reported by ErrorProne in the main API, REST-API (old one), and OAI-PMH. I've included each module's fixes as a separate commit so that they can potentially be backported (as necessary) to 6.x.

None of the bugs seem significant (and there aren't too many overall). But, ErrorProne definitely found some undiscovered issues in our codebase. Here's just a few patterns that were discovered (and fixed):

@tdonohue tdonohue changed the title Add Error Prone checks to catch common Java code mistakes DS-3712: Add Error Prone checks to catch common Java code mistakes Oct 5, 2017

@tdonohue

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2017

Looks like ErrorProne also checks Unit Test classes for common mistakes. The initial build failed here cause it found this mistake: http://errorprone.info/bugpattern/JUnit4TestNotRun

But, this gives a good example of how ErrorProne responds in practice. Here's the full detail ErrorProne gives about the problem, including a "Did you mean?" code example:
https://travis-ci.org/DSpace/DSpace/builds/283800202#L1848

A fix to this new issue coming shortly

@tdonohue

This comment has been minimized.

Copy link
Member Author

commented Oct 5, 2017

PR is fixed and ErrorProne / Travis has given a full green light. Ready for review!

@hardyoyo
Copy link
Member

left a comment

I like this idea, +1

@tdonohue tdonohue added this to the 7.0 milestone Oct 5, 2017

@terrywbrady terrywbrady merged commit 48aa148 into DSpace:master Oct 11, 2017

1 check passed

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

@tdonohue tdonohue deleted the tdonohue:add_error_prone_checks branch Oct 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.