-
Notifications
You must be signed in to change notification settings - Fork 118
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
Implementation of #113 Plugin Facility for Sputnik Processors #131
Conversation
Current coverage is
|
if (Boolean.valueOf(configuration.getProperty(GeneralOption.SONAR_ENABLED))) { | ||
processors.add(new SonarProcessor(configuration)); | ||
|
||
ServiceLoader<ReviewProcessorFactory> theLoader = ServiceLoader.load(ReviewProcessorFactory.class); |
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'd change variable names to match other variables style, so: serviceLoader instead of theLoader, iterator instead of theIterator, reviewProcessorFactory instead of theFactory etc.
Can you elaborate on this? As I understand your work you want to use ServiceLoader to locate all classes that implement interface ReviewProcessorFactory and are enabled in properties file? This way you can write external plugins that are not included in Sputnik codebase. Is this correct? It's hard to unit test your change. Do you have an idea how to do this? |
Yes, you are right. Now custom ReviewProcessor can be added using the standard Java ServiceLoader API and extending the Classpath. This enables user to implement custom ReviewProcessors without affecting the codebase and adding unwanted dependencies to the core project. UnitTesting this change is quite easy. Just call the ProcessorBuilder and see it if returns more than zero ReviewProcessor instances(with the right configuration). |
Added Unit Tests
Fixed code style issues
import pl.touk.sputnik.configuration.Configuration; | ||
import pl.touk.sputnik.configuration.GeneralOption; | ||
import pl.touk.sputnik.processor.ReviewProcessorFactory; | ||
import pl.touk.sputnik.processor.checkstyle.CheckstyleReviewProcessorFactory; |
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.
[Checkstyle] ERROR: Unused import - pl.touk.sputnik.processor.checkstyle.CheckstyleReviewProcessorFactory.
Fixed code style issues
Fixed the codestyle issues and added some tests. |
Any news on this? |
Implementation of #113 Plugin Facility for Sputnik Processors
Thank you! Merged! |
Implementation of TouK#113 Plugin Facility for Sputnik Processors
First shot for an implementation based on the Java ServiceLoader mechanism.