You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/15 13:28:32 UTC
[GitHub] [maven-plugin-tools] slawekjaranowski opened a new pull request, #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
slawekjaranowski opened a new pull request, #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151
- new implementation
- remove some of unused or unneeded methods
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] kwin commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1283891880
> @kwin thanks for approve. Please look again - I rebased code with your last change.
Still looks good ;-)
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] slawekjaranowski commented on a diff in pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#discussion_r997498036
##########
maven-plugin-plugin/src/site/apt/examples/using-annotations.apt.vm:
##########
@@ -41,18 +41,22 @@ Using Plugin Tools Java Annotations
Information for plugin descriptor generation is specified using 4 annotations:
- * 2 class-level annotations:
+ * 2 class level annotations:
* <<<...@Mojo>>>: This annotation will mark your class as a Mojo,
* <<<...@Execute>>>: Used if your Mojo needs to fork a lifecycle,
[]
- * 2 field-level annotations:
+ * 1 filed or method level annotations:
Review Comment:
ups ... I forgot to push ... now should be visible 😄
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] gnodet commented on a diff in pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
gnodet commented on code in PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#discussion_r999518801
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/scanner/MojoAnnotationsScanner.java:
##########
@@ -45,6 +45,9 @@
Component.class.getName(),
Deprecated.class.getName() );
+ List<String> METHOD_LEVEL_ANNOTATIONS = Arrays.asList( Parameter.class.getName(),
+ Deprecated.class.getName() );
Review Comment:
Column alignment for `Deprecated` ?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] slawekjaranowski commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1284431737
> Looks good (apart from the very minor indentation issue). Some of it had already been integrated in the v4 api through [gnodet/maven@6317b8c](https://github.com/gnodet/maven/commit/6317b8c0ad468a4abdee47531d461755ff051d0a), but both `@Parameter` and `@Component` annotations were actually modified. Does it make sense or should I revert the changes on `@Component` ?
We can add support for `@Compoent` with method in next step.
I must check if old `@Compoent` can be injected by method - maybe you know it.
I hope we can also add support for old and new annotation together.
As soon Maven 4 alpha will be published we will have open way for it.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] gnodet commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1284497850
> > Looks good (apart from the very minor indentation issue). Some of it had already been integrated in the v4 api through [gnodet/maven@6317b8c](https://github.com/gnodet/maven/commit/6317b8c0ad468a4abdee47531d461755ff051d0a), but both `@Parameter` and `@Component` annotations were actually modified. Does it make sense or should I revert the changes on `@Component` ?
>
> We can add support for `@Compoent` with method in next step. I must check if old `@Compoent` can be injected by method - maybe you know it.
I don't think so at the moment. The question is more whether sisu-inject supports injecting components using setters, but I would think so.
> I hope we can also add support for old and new annotation together. As soon Maven 4 alpha will be published we will have open way for it.
That's done in https://github.com/apache/maven-plugin-tools/pull/117 . Both annotation packages are supported. It's ready for review (may need to rebase based on recent changes), but I think it would be better in a new major version of the plugin tools.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] slawekjaranowski commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1281355097
> Would be good to update https://maven.apache.org/guides/plugin/guide-java-plugin-development.html#parameters and https://maven.apache.org/developers/mojo-api-specification.html subsequently.
Agree - documentation should be updated .... we need to review it after release
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] kwin commented on a diff in pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#discussion_r997488021
##########
maven-plugin-plugin/src/site/apt/examples/using-annotations.apt.vm:
##########
@@ -41,18 +41,22 @@ Using Plugin Tools Java Annotations
Information for plugin descriptor generation is specified using 4 annotations:
- * 2 class-level annotations:
+ * 2 class level annotations:
* <<<...@Mojo>>>: This annotation will mark your class as a Mojo,
* <<<...@Execute>>>: Used if your Mojo needs to fork a lifecycle,
[]
- * 2 field-level annotations:
+ * 1 filed or method level annotations:
Review Comment:
How was this and my other remarks solved? I don't see a new commit...
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] gnodet commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1284270701
Looks good (apart from the very minor indentation issue).
Some of it had already been integrated in the v4 api through https://github.com/gnodet/maven/commit/6317b8c0ad468a4abdee47531d461755ff051d0a, but both `@Parameter` and `@Component` annotations were actually modified. Does it make sense or should I revert the changes on `@Component` ?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] kwin commented on a diff in pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#discussion_r996966815
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -395,32 +397,33 @@ private DocletTag findInClassHierarchy( JavaClass javaClass, String tagName )
/**
* extract fields that are either parameters or components.
+ * also extract methods that are parameters
Review Comment:
Upper case "Also".
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -395,32 +397,33 @@ private DocletTag findInClassHierarchy( JavaClass javaClass, String tagName )
/**
* extract fields that are either parameters or components.
+ * also extract methods that are parameters
*
* @param javaClass not null
* @return map with Mojo parameters names as keys
*/
- private Map<String, JavaField> extractFieldParameterTags( JavaClass javaClass,
- Map<String, JavaClass> javaClassesMap )
+ private Map<String, JavaAnnotatedElement> extractParameterTags( JavaClass javaClass,
Review Comment:
I would rename this method `extractParameterAnnotations`. Tag is such an overloaded term.
##########
maven-plugin-tools-annotations/src/main/java/org/apache/maven/tools/plugin/extractor/annotations/JavaAnnotationsMojoDescriptorExtractor.java:
##########
@@ -443,6 +455,15 @@ private Map<String, JavaField> extractFieldParameterTags( JavaClass javaClass,
}
}
+ private boolean isPublicSetterMethod( JavaMethod method )
+ {
+ return method.isPublic()
Review Comment:
Sisu also injects into methods with prefix "add". Also you need to check for the modifier (https://github.com/eclipse/sisu.plexus/blob/27a29dc633e6b03753a3c8d29a033648630c8831/org.eclipse.sisu.plexus/src/org/eclipse/sisu/plexus/CompositeBeanHelper.java#L143-L147 and https://github.com/eclipse/sisu.plexus/blob/27a29dc633e6b03753a3c8d29a033648630c8831/org.eclipse.sisu.plexus/src/org/eclipse/sisu/plexus/CompositeBeanHelper.java#L267-L282)
##########
maven-plugin-plugin/src/site/apt/examples/using-annotations.apt.vm:
##########
@@ -41,18 +41,22 @@ Using Plugin Tools Java Annotations
Information for plugin descriptor generation is specified using 4 annotations:
- * 2 class-level annotations:
+ * 2 class level annotations:
* <<<...@Mojo>>>: This annotation will mark your class as a Mojo,
* <<<...@Execute>>>: Used if your Mojo needs to fork a lifecycle,
[]
- * 2 field-level annotations:
+ * 1 filed or method level annotations:
Review Comment:
filed -> field
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] slawekjaranowski merged pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] slawekjaranowski commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1282330822
@kwin thanks for approve.
Please look again - I rebased code with your last change.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-plugin-tools] kwin commented on pull request #151: [MPLUGIN-419] Allow `@Parameter` on setters methods
Posted by GitBox <gi...@apache.org>.
kwin commented on PR #151:
URL: https://github.com/apache/maven-plugin-tools/pull/151#issuecomment-1280764061
Would be good to update https://maven.apache.org/guides/plugin/guide-java-plugin-development.html#parameters subsequently.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org