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 2023/01/09 17:56:34 UTC

[GitHub] [maven-integration-testing] psiroky opened a new pull request, #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

psiroky opened a new pull request, #226:
URL: https://github.com/apache/maven-integration-testing/pull/226

   @slawekjaranowski thanks for merging the other PRs! This is another one - slightly smaller than the the last one, but still pretty big (even though 95% of the changes were again done automatically with find&replace).
   


-- 
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-integration-testing] slawekjaranowski merged pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226


-- 
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-integration-testing] slawekjaranowski commented on pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226#issuecomment-1381466894

   First I want to merge: https://github.com/apache/maven/pull/953 to be sure that tests are ok.


-- 
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-integration-testing] psiroky commented on pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226#issuecomment-1381463498

   @cstamas no worries, the conflict was trivial to resolve. Should be ready now.


-- 
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-integration-testing] psiroky commented on a diff in pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226#discussion_r1066959339


##########
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4412OfflineModeInPluginTest.java:
##########
@@ -93,10 +93,10 @@ public void testitCollector()
         verifier.deleteDirectory( "target" );
         verifier.deleteArtifacts( "org.apache.maven.its.mng4412" );
         verifier.filterFile( "settings-template.xml", "settings.xml", "UTF-8", verifier.newDefaultFilterProperties() );
-        verifier.addCliOption( "-Pcollector" );
-        verifier.addCliOption( "--offline" );
-        verifier.addCliOption( "-s" );
-        verifier.addCliOption( "settings.xml" );
+        verifier.addCliArgument( "-Pcollector" );
+        verifier.addCliArgument( "--offline" );
+        verifier.addCliArgument( "-s" );
+        verifier.addCliArgument( "settings.xml" );

Review Comment:
   These successive calls to `addCliArgument` could be possibly replaced by single call to
   ```
   addCliArguments("-Pcollector", "--offline", "-s", "settings.xml");
   ```
   I am not sure though if that's making things easier or harder to read (it may be just matter of preference). In any case, those would be done in different PR anyway.



##########
core-it-suite/src/test/java/org/apache/maven/it/MavenITmng4412OfflineModeInPluginTest.java:
##########
@@ -93,10 +93,10 @@ public void testitCollector()
         verifier.deleteDirectory( "target" );
         verifier.deleteArtifacts( "org.apache.maven.its.mng4412" );
         verifier.filterFile( "settings-template.xml", "settings.xml", "UTF-8", verifier.newDefaultFilterProperties() );
-        verifier.addCliOption( "-Pcollector" );
-        verifier.addCliOption( "--offline" );
-        verifier.addCliOption( "-s" );
-        verifier.addCliOption( "settings.xml" );
+        verifier.addCliArgument( "-Pcollector" );
+        verifier.addCliArgument( "--offline" );
+        verifier.addCliArgument( "-s" );
+        verifier.addCliArgument( "settings.xml" );

Review Comment:
   These successive calls to `addCliArgument` could be possibly replaced by a single call to
   ```
   addCliArguments("-Pcollector", "--offline", "-s", "settings.xml");
   ```
   I am not sure though if that's making things easier or harder to read (it may be just matter of preference). In any case, those would be done in different PR anyway.



-- 
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-integration-testing] cstamas commented on pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226#issuecomment-1381428841

   @psiroky sorry for causing inconvenience, and I am aware this is 2nd time, but please can you fix the conflict? :smile: and then let's merge @slawekjaranowski 


-- 
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-integration-testing] gnodet commented on pull request #226: [MNG-7661] Replace deprecated 'verifier.addCliOption()'

Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #226:
URL: https://github.com/apache/maven-integration-testing/pull/226#issuecomment-1378693835

   @psiroky could you fix the conflict please ?


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