FINERACT-696 - add spotbug bug-pattern plugin #716
Conversation
xurror
force-pushed
the
FINERACT-696
branch
2 times, most recently
from
f22d8a8
to
06501e4
Compare
xurror
changed the title
xurror
force-pushed
the
FINERACT-696
branch
2 times, most recently
from
a4a5c26
to
8320b04
Compare
|
Looks like you have a spotbugs violation and possibly a Checkstyle voilation in this PR. Opening and closing n times may not solve this. run ./gradlew clean check to see the problem and fix them on local Dev env before sending PR: See: https://travis-ci.org/apache/fineract/builds/658740697#L1963 You can also check the logs online via Travis.. Do you understand what I mean? |
|
This still fails due to a hopefully simple to fix SpotBugs violation problem. This failure has nothing to do with the problems we are chasing in https://issues.apache.org/jira/browse/FINERACT-850 and sub-tasks. |
|
@xurror actually I suspect that this probably now fails SpotBugs because it detects (many?) wrong log statements? We should just fix them all up, as part of this PR... I know it's a bit painful manual work, but "fixing all and immediately enforcing" is the only pattern that I know of to make something like this have any real lasting impact (similar to #715). |
xurror
force-pushed
the
FINERACT-696
branch
2 times, most recently
from
aba151a
to
f8e6f1f
Compare
|
@vorburger I already resolved them earlier today, it was actually quite long and boring but I think it's ready for merge |
...rovider/src/main/java/org/apache/fineract/commands/service/AuditReadPlatformServiceImpl.java
Outdated
Show resolved
Hide resolved
...act/infrastructure/campaigns/email/service/EmailCampaignWritePlatformCommandHandlerImpl.java
Outdated
Show resolved
Hide resolved
...he/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java
Outdated
Show resolved
Hide resolved
...apache/fineract/infrastructure/entityaccess/service/FineractEntityAccessReadServiceImpl.java
Outdated
Show resolved
Hide resolved
...java/org/apache/fineract/infrastructure/sms/scheduler/SmsMessageScheduledJobServiceImpl.java
Outdated
Show resolved
Hide resolved
xurror
force-pushed
the
FINERACT-696
branch
2 times, most recently
from
d87fa21
to
a543d0a
Compare
|
@vorburger,this is ready |
add spotbug bug-pattern plugin. Apply suggestions from code review closes https://issues.apache.org/jira/browse/FINERACT-696 Co-Authored-By: Michael Vorburger ⛑️ <mike@vorburger.ch>
Description
While code reviewing #464, I came accross a catch & printStackTrace, which is of course wrong (it should instead be, correctly, logged, instead).
In an ideal world, it would not take human code review to catch this, but just an automated build failure. This is possible using SpotBugs (not FindBugs anymore; SpotBugs is the new FindBugs, it's a fully compatible successor) and https://github.com/KengoTODA/findbugs-slf4j/ (for this particular case KengoTODA/findbugs-slf4j#70, but using findbugs-slf4j for Fineract would have value even before that's implemented).
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
Commit message starts with the issue number from https://issues.apache.org/jira/projects/FINERACT/. Ex: FINERACT-646 Pockets API.
Coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions have been followed.
API documentation at https://github.com/apache/fineract/blob/develop/api-docs/apiLive.htm has been updated with details of any API changes.
Integration tests have been created/updated for verifying the changes made.
All Integrations tests are passing with the new commits.
Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the list.)
Our guidelines for code reviews is at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide