gnodet-bot commented on code in PR #10:
URL:
https://github.com/apache/maven-deploy-plugin/pull/10#discussion_r4163252828
##########
src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java:
##########
@@ -140,7 +146,10 @@ public void execute()
throws MojoExecutionException, MojoFailureException
{
boolean addedDeployRequest = false;
- if ( skip )
+ if ( Boolean.parseBoolean( skip )
+ || ( "releases".equals( skip ) && !ArtifactUtils.isSnapshot(
project.getVersion() ) )
+ || ( "snapshots".equals( skip ) && ArtifactUtils.isSnapshot(
project.getVersion() ) )
Review Comment:
⚠️ **Case sensitivity inconsistency:** `"releases".equals(skip)` and
`"snapshots".equals(skip)` are case-sensitive, but `Boolean.parseBoolean(skip)`
is case-insensitive (accepts `TRUE`, `True`, etc.). A user writing
`-Dmaven.deploy.skip=Releases` or `-Dmaven.deploy.skip=SNAPSHOTS` will silently
get no skip — the value falls through to `Boolean.parseBoolean()` which returns
`false`.
Use `equalsIgnoreCase` for consistency:
```suggestion
if ( Boolean.parseBoolean( skip )
|| ( "releases".equalsIgnoreCase( skip ) &&
!ArtifactUtils.isSnapshot( project.getVersion() ) )
|| ( "snapshots".equalsIgnoreCase( skip ) &&
ArtifactUtils.isSnapshot( project.getVersion() ) )
```
##########
src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java:
##########
@@ -124,11 +124,17 @@ public class DeployMojo
/**
* Set this to 'true' to bypass artifact deploy
- *
+ * Since since 3.0.0-M2 it's not anymore a real boolean as it can have
more than 2 values:
+ * <ul>
+ * <li><code>true</code>: will skip as usual</li>
+ * <li><code>releases</code>: will skip if current version of the
project is a release</li>
+ * <li><code>snapshots</code>: will skip if current version of the
project is a snapshot</li>
+ * <li>any other values will be considered as <code>false</code></li>
+ * </ul>
Review Comment:
🔸 **Javadoc:** Missing blank line before `@since` tag — standard Javadoc
convention separates the description block from tags with an empty line.
```suggestion
* <li>any other values will be considered as <code>false</code></li>
* </ul>
*
```
##########
src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java:
##########
@@ -124,11 +124,17 @@ public class DeployMojo
/**
* Set this to 'true' to bypass artifact deploy
- *
+ * Since since 3.0.0-M2 it's not anymore a real boolean as it can have
more than 2 values:
Review Comment:
🔸 **Javadoc typo:** "Since since 3.0.0-M2" — duplicated word.
```suggestion
* Since 3.0.0-M2 it's not anymore a real boolean as it can have more
than 2 values:
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
