[GitHub] [maven] MartinKanters commented on a diff in pull request #741: [MNG-7468] Check unsupported plugins parameters in configuration

[GitBox <gi...@apache.org>]

MartinKanters commented on code in PR #741:
URL: https://github.com/apache/maven/pull/741#discussion_r873452132


##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java:
##########
@@ -358,6 +368,61 @@ private void finalizeMojoConfiguration( MojoExecution mojoExecution )
         mojoExecution.setConfiguration( finalConfiguration );
     }
 
+    private void checkUnKnownMojoConfigurationParameters( MojoExecution mojoExecution )
+        throws PluginConfigurationException
+    {
+        if ( mojoExecution.getConfiguration() == null || mojoExecution.getConfiguration().getChildCount() == 0 )
+        {
+            return;
+        }
+
+        // first stem get parameter names of current goal
+        Set<String> parametersNames = new HashSet<>();
+        for ( Parameter p : mojoExecution.getMojoDescriptor().getParameters() )

Review Comment:
   I unfortunately do not have the time to do a proper review, but I noticed some things at a glance:
   - I believe this for loop and the one on line 404 is the same. Have you considered refactoring that to a method? 
   - If you do the first point, then this method can all be done in a couple of streams instead of for-loops alternated with streams. Could be a small styling benefit. 
   - typo in the comment on line 379 (step*) 
   - 



##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/DefaultLifecycleExecutionPlanCalculator.java:
##########
@@ -358,6 +368,61 @@ private void finalizeMojoConfiguration( MojoExecution mojoExecution )
         mojoExecution.setConfiguration( finalConfiguration );
     }
 
+    private void checkUnKnownMojoConfigurationParameters( MojoExecution mojoExecution )
+        throws PluginConfigurationException
+    {
+        if ( mojoExecution.getConfiguration() == null || mojoExecution.getConfiguration().getChildCount() == 0 )
+        {
+            return;
+        }
+
+        // first stem get parameter names of current goal
+        Set<String> parametersNames = new HashSet<>();
+        for ( Parameter p : mojoExecution.getMojoDescriptor().getParameters() )

Review Comment:
   I unfortunately do not have the time to do a proper review, but I noticed some things at a glance:
   - I believe this for loop and the one on line 404 is the same. Have you considered refactoring that to a method? 
   - If you do the first point, then this method can all be done in a couple of streams instead of for-loops alternated with streams. Could be a small styling benefit. 
   - typo in the comment on line 379 (step*) 



-- 
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