-
Notifications
You must be signed in to change notification settings - Fork 53
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
Get rid of Reflections in PluginCompatTesterHooks
#481
Conversation
<groupId>org.kohsuke.metainf-services</groupId> | ||
<artifactId>metainf-services</artifactId> | ||
<version>1.9</version> |
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.
Needed to create the META-INF/services
entries for these hooks to be loadable via ServiceLoader
.
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-compiler-plugin</artifactId> | ||
<!-- Version specified in parent POM --> | ||
<configuration> | ||
<annotationProcessorPaths> | ||
<path> | ||
<groupId>info.picocli</groupId> | ||
<artifactId>picocli-codegen</artifactId> | ||
<version>${picocli.version}</version> | ||
</path> | ||
</annotationProcessorPaths> | ||
<compilerArgs> | ||
<arg>-Aproject=${project.groupId}/${project.artifactId}</arg> | ||
</compilerArgs> | ||
</configuration> | ||
</plugin> |
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 was interfering with the registration of the metainf-services
annotation processor. Since it was optional, I opted to dispense with it rather than debug it.
@CheckForNull | ||
@CommandLine.Option( | ||
names = "--hook-prefixes", | ||
split = ",", | ||
arity = "1", | ||
paramLabel = "prefix", | ||
description = "Comma-separated list of prefixes of extra hook classes.") | ||
private List<String> hookPrefixes; | ||
|
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.
No longer makes sense now that an annotation is the explicit way to opt in.
Fixes #394 by dropping the dependency on Reflections in favor of
ServiceLoader
. This requires each hook to be annotated with@MetaInfServices(PluginCompatTesterHookBeforeCheckout.class)
,@MetaInfServices(PluginCompatTesterHookBeforeCompile.class)
, or@MetaInfServices(PluginCompatTesterHookBeforeExecution.class)
and obviates the need for a--hook-prefixes
option. To test this I verified in the debugger that the same hooks got loaded as before this PR (in the same order).