You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "timtebeek (via GitHub)" <gi...@apache.org> on 2023/05/22 15:28:55 UTC
[GitHub] [maven-jarsigner] timtebeek opened a new pull request, #3: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
timtebeek opened a new pull request, #3:
URL: https://github.com/apache/maven-jarsigner/pull/3
Use this link to re-run the recipe: https://public.moderne.io/recipes/org.openrewrite.java.migrate.apache.commons.lang.IsNotEmptyToJdk?organizationId=QXBhY2hlIE1hdmVu
--
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-jarsigner] timtebeek commented on a diff in pull request #3: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #3:
URL: https://github.com/apache/maven-jarsigner/pull/3#discussion_r1202293944
##########
src/main/java/org/apache/maven/shared/jarsigner/JarSignerCommandLineBuilder.java:
##########
@@ -82,28 +82,28 @@ public Commandline build( JarSignerRequest request )
}
String storetype = request.getStoretype();
- if ( !StringUtils.isEmpty( storetype ) )
+ if ( !(storetype == null || storetype.isEmpty()) )
{
cli.createArg().setValue( "-storetype" );
cli.createArg().setValue( storetype );
}
String providerName = request.getProviderName();
- if ( !StringUtils.isEmpty( providerName ) )
+ if ( !(providerName == null || providerName.isEmpty()) )
{
cli.createArg().setValue( "-providerName" );
cli.createArg().setValue( providerName );
}
String providerClass = request.getProviderClass();
- if ( !StringUtils.isEmpty( providerClass ) )
+ if ( !(providerClass == null || providerClass.isEmpty()) )
Review Comment:
No it's a bit unfortunate; would have required a separate recipe (or targeted manual change) to first replace the `!StringUtils.isEmpty` with `StringUtils.isNotEmpty`. But then since we are phasing these out we decided against it on an earlier pull request.
In this specific case we can fix these manually here if you'd like, as I imagine checkstyle will have its complaints as well.
--
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-jarsigner] timtebeek commented on a diff in pull request #3: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #3:
URL: https://github.com/apache/maven-jarsigner/pull/3#discussion_r1202293944
##########
src/main/java/org/apache/maven/shared/jarsigner/JarSignerCommandLineBuilder.java:
##########
@@ -82,28 +82,28 @@ public Commandline build( JarSignerRequest request )
}
String storetype = request.getStoretype();
- if ( !StringUtils.isEmpty( storetype ) )
+ if ( !(storetype == null || storetype.isEmpty()) )
{
cli.createArg().setValue( "-storetype" );
cli.createArg().setValue( storetype );
}
String providerName = request.getProviderName();
- if ( !StringUtils.isEmpty( providerName ) )
+ if ( !(providerName == null || providerName.isEmpty()) )
{
cli.createArg().setValue( "-providerName" );
cli.createArg().setValue( providerName );
}
String providerClass = request.getProviderClass();
- if ( !StringUtils.isEmpty( providerClass ) )
+ if ( !(providerClass == null || providerClass.isEmpty()) )
Review Comment:
No it's a bit unfortunate; would have required a separate recipe (or targeted manual change) to first replace the `!StringUtils.isEmpty` with `StringUtils.isNotEmpty`. But then since we are phasing these out we decided against it on an earlier pull request.
In this specific case we can fix these manually here if you'd like, as I imagine checkstyle will have it's complaints as well.
--
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-jarsigner] elharo commented on a diff in pull request #3: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #3:
URL: https://github.com/apache/maven-jarsigner/pull/3#discussion_r1202282976
##########
src/main/java/org/apache/maven/shared/jarsigner/JarSignerCommandLineBuilder.java:
##########
@@ -82,28 +82,28 @@ public Commandline build( JarSignerRequest request )
}
String storetype = request.getStoretype();
- if ( !StringUtils.isEmpty( storetype ) )
+ if ( !(storetype == null || storetype.isEmpty()) )
{
cli.createArg().setValue( "-storetype" );
cli.createArg().setValue( storetype );
}
String providerName = request.getProviderName();
- if ( !StringUtils.isEmpty( providerName ) )
+ if ( !(providerName == null || providerName.isEmpty()) )
{
cli.createArg().setValue( "-providerName" );
cli.createArg().setValue( providerName );
}
String providerClass = request.getProviderClass();
- if ( !StringUtils.isEmpty( providerClass ) )
+ if ( !(providerClass == null || providerClass.isEmpty()) )
Review Comment:
I guess the recipe doesn't pick up the ! in StringUtils.isEmpty( providerClass )? This isn't the same replacement as StringUtils.isNotEmpty.
--
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-jarsigner] elharo merged pull request #3: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo merged PR #3:
URL: https://github.com/apache/maven-jarsigner/pull/3
--
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