You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2025/11/28 19:25:25 UTC

[PR] Replace deprecated setGoals method [maven-release]

elharo opened a new pull request, #1441:
URL: https://github.com/apache/maven-release/pull/1441

   (no comment)


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


Re: [PR] Replace deprecated setGoals method [maven-release]

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #1441:
URL: https://github.com/apache/maven-release/pull/1441#discussion_r2572595797


##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java:
##########
@@ -113,7 +113,7 @@ public void executeGoals(
                 targetGoals.add(additionalArguments);
             }
 
-            req.setGoals(targetGoals);
+            req.addArgs(targetGoals);

Review Comment:
   This is what the deprecation message says:
   
   ```
     /**
        * Sets the goals for the Maven invocation.
        *
        * @param goals The goals for the Maven invocation, may be <code>null</code> to execute the POMs default goal.
        * @return This invocation request.
        * @deprecated simply {@link #addArg(String)} or {@link #addArgs(Collection)} should be used
        */
   ```
   
   Is that wrong then? Should this method be undeprecated?
   



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


Re: [PR] Replace deprecated setGoals method [maven-release]

Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz closed pull request #1441: Replace deprecated setGoals method
URL: https://github.com/apache/maven-release/pull/1441


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


Re: [PR] Replace deprecated setGoals method [maven-release]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1441:
URL: https://github.com/apache/maven-release/pull/1441#discussion_r2572610077


##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java:
##########
@@ -113,7 +113,7 @@ public void executeGoals(
                 targetGoals.add(additionalArguments);
             }
 
-            req.setGoals(targetGoals);
+            req.addArgs(targetGoals);

Review Comment:
   It is correctly deprecated ... but when we use new method we need new parameters where user set each command line argument separately
   
   Parsing, escaping arguments are allwaye error prone, so we should required from users to provided 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


Re: [PR] Replace deprecated setGoals method [maven-release]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #1441:
URL: https://github.com/apache/maven-release/pull/1441#discussion_r2572863846


##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java:
##########
@@ -113,7 +113,7 @@ public void executeGoals(
                 targetGoals.add(additionalArguments);
             }
 
-            req.setGoals(targetGoals);
+            req.addArgs(targetGoals);

Review Comment:
   Correct,  don't reinvent the wheel you will fail.



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


Re: [PR] Replace deprecated setGoals method [maven-release]

Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1441:
URL: https://github.com/apache/maven-release/pull/1441#discussion_r2572568384


##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/exec/InvokerMavenExecutor.java:
##########
@@ -113,7 +113,7 @@ public void executeGoals(
                 targetGoals.add(additionalArguments);
             }
 
-            req.setGoals(targetGoals);
+            req.addArgs(targetGoals);

Review Comment:
   It is not a good idea. Each argument passed to `setGoals` are internal parsed, escaped and so on ...
    but `addArgs` pass arguments as is to command line process
   
   look at line 113 `additionalArguments` is add as one item to list



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