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:29:56 UTC
[GitHub] [maven-install-plugin] timtebeek opened a new pull request, #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
timtebeek opened a new pull request, #58:
URL: https://github.com/apache/maven-install-plugin/pull/58
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-install-plugin] elharo commented on a diff in pull request #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #58:
URL: https://github.com/apache/maven-install-plugin/pull/58#discussion_r1203920552
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -218,7 +218,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
ArtifactType artifactType =
repositorySystemSession.getArtifactTypeRegistry().get(packaging);
if (artifactType != null
- && StringUtils.isEmpty(classifier)
+ && (classifier == null || classifier.isEmpty())
&& !StringUtils.isEmpty(artifactType.getClassifier())) {
Review Comment:
Noticed it missed one in line 222
--
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-install-plugin] timtebeek commented on a diff in pull request #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #58:
URL: https://github.com/apache/maven-install-plugin/pull/58#discussion_r1212983344
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -218,7 +218,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
ArtifactType artifactType =
repositorySystemSession.getArtifactTypeRegistry().get(packaging);
if (artifactType != null
- && StringUtils.isEmpty(classifier)
+ && (classifier == null || classifier.isEmpty())
&& !StringUtils.isEmpty(artifactType.getClassifier())) {
Review Comment:
Looking at this again: the line was not missed, but intentionally left out in the recipe, as we can't be certain that the argument method invocation is safe to call twice; once for the null check, and again for the isEmpty call. Hence why I don't convert those automatically yet, even though in this case it's probably safe to do.
--
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-install-plugin] timtebeek commented on pull request #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on PR #58:
URL: https://github.com/apache/maven-install-plugin/pull/58#issuecomment-1559140528
@elharo I notice that my commit message didn't make it into the created pull request message, so you might not be aware of some of these last open pull requests. Would you mind going through [this last batch](https://github.com/pulls?q=is%3Aopen+is%3Apr+archived%3Afalse+%22mng-6829%22+author%3Atimtebeek)?
> Last batch of is(Not)Empty for https://issues.apache.org/jira/browse/MNG-6829
These are the smallest change sets, hence why I opened more at the same time.
After this we can target the next most often used method from the StringUtils classes.
--
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-install-plugin] timtebeek commented on a diff in pull request #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "timtebeek (via GitHub)" <gi...@apache.org>.
timtebeek commented on code in PR #58:
URL: https://github.com/apache/maven-install-plugin/pull/58#discussion_r1203931524
##########
src/main/java/org/apache/maven/plugins/install/InstallFileMojo.java:
##########
@@ -218,7 +218,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
ArtifactType artifactType =
repositorySystemSession.getArtifactTypeRegistry().get(packaging);
if (artifactType != null
- && StringUtils.isEmpty(classifier)
+ && (classifier == null || classifier.isEmpty())
&& !StringUtils.isEmpty(artifactType.getClassifier())) {
Review Comment:
That's surprising! Usually when that happens there's some missing type attribution; I'll do a final sweep to see if it pops up again.
--
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-install-plugin] elharo merged pull request #58: [MNG-6829] Replace StringUtils#isEmpty(String) & #isNotEmpty(String)
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo merged PR #58:
URL: https://github.com/apache/maven-install-plugin/pull/58
--
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