michael-o commented on code in PR #236:
URL:
https://github.com/apache/maven-plugin-tools/pull/236#discussion_r1382595128
##########
maven-plugin-tools-annotations/src/site/apt/index.apt:
##########
@@ -94,27 +94,42 @@ public class MyMojo
hint = "..." )
private MyComponent component;
+ // pseudo-parameters (marked read-only) permitting injection of Maven
build context objects
// sample objects taken from Maven API through
PluginParameterExpressionEvaluator
+ //
https://maven.apache.org/ref/current/maven-core/apidocs/org/apache/maven/plugin/PluginParameterExpressionEvaluator.html
+ // plugins targetting Maven 3.2.5+ (after MNG-5695) should not use these
pseudo-parameters any more,
+ // but @Component and Maven APIs to get better compiler-time checks
- @Parameter( defaultValue = "${session}", readonly = true )
+ // @Parameter( defaultValue = "${session}", readonly = true )
+ @Component // since Maven 3.2.5, thanks to MNG-5695
Review Comment:
> > > We were discussing and leaning to the exact opposite. Keep
`@Parameter` for things injected from the config and `@Component/@Inject` for
injecting beans, including those from the session (Session) and mojoExecution
scopes (MojoExecution, Project)...
> >
> >
> > This is exactly what I remember. Use `@Parameter` exactly for this case.
>
> But those components can also be injected with `@Component`, so this is
weird to warn about something that is supported by sisu (because those are
actually bound to scopes), while pushing users to a _custom_ solution. Why only
a few components would be accessible by name then ? I would agree if we had a
single annotation for both use cases, but that's not the case in Maven 3. I
suppose we could try for the v4 api though.
I agree that this is inconsistent. If both are valid and are functionally
identical then the warning is warong.
##########
maven-plugin-tools-annotations/src/site/apt/index.apt:
##########
@@ -94,27 +94,42 @@ public class MyMojo
hint = "..." )
private MyComponent component;
+ // pseudo-parameters (marked read-only) permitting injection of Maven
build context objects
// sample objects taken from Maven API through
PluginParameterExpressionEvaluator
+ //
https://maven.apache.org/ref/current/maven-core/apidocs/org/apache/maven/plugin/PluginParameterExpressionEvaluator.html
+ // plugins targetting Maven 3.2.5+ (after MNG-5695) should not use these
pseudo-parameters any more,
+ // but @Component and Maven APIs to get better compiler-time checks
- @Parameter( defaultValue = "${session}", readonly = true )
+ // @Parameter( defaultValue = "${session}", readonly = true )
+ @Component // since Maven 3.2.5, thanks to MNG-5695
Review Comment:
> > > We were discussing and leaning to the exact opposite. Keep
`@Parameter` for things injected from the config and `@Component/@Inject` for
injecting beans, including those from the session (Session) and mojoExecution
scopes (MojoExecution, Project)...
> >
> >
> > This is exactly what I remember. Use `@Parameter` exactly for this case.
>
> But those components can also be injected with `@Component`, so this is
weird to warn about something that is supported by sisu (because those are
actually bound to scopes), while pushing users to a _custom_ solution. Why only
a few components would be accessible by name then ? I would agree if we had a
single annotation for both use cases, but that's not the case in Maven 3. I
suppose we could try for the v4 api though.
I agree that this is inconsistent. If both are valid and are functionally
identical then the warning is wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
