[GitHub] [maven-resolver] michael-o commented on a change in pull request #119: Simplify conditions with the same result and avoid extra validations.

[GitBox <gi...@apache.org>]

michael-o commented on a change in pull request #119:
URL: https://github.com/apache/maven-resolver/pull/119#discussion_r702318011



##########
File path: maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/ChecksumCalculator.java
##########
@@ -153,12 +153,8 @@ public void init( long dataOffset )
                 int read = in.read( array );
                 if ( read < 0 )
                 {
-                    if ( total < dataOffset )
-                    {
-                        throw new IOException( targetFile + " contains only " + total
-                                                   + " bytes, cannot resume download from offset " + dataOffset );
-                    }
-                    break;
+                    throw new IOException( targetFile + " contains only " + total

Review comment:
       Can you explain this change?

##########
File path: maven-resolver-api/src/main/java/org/eclipse/aether/resolution/ArtifactResolutionException.java
##########
@@ -43,7 +43,7 @@
     public ArtifactResolutionException( List<ArtifactResult> results )
     {
         super( getMessage( results ), getCause( results ) );
-        this.results = ( results != null ) ? results : Collections.<ArtifactResult>emptyList();
+        this.results = results;

Review comment:
       This actually changes semantics b/c if `results` is null the instance private field wll never be null. WDYT?

##########
File path: maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/manager/ClassicDependencyManager.java
##########
@@ -177,10 +177,7 @@ public DependencyManagement manageDependency( Dependency dependency )
             String version = managedVersions.get( key );
             if ( version != null )
             {
-                if ( management == null )
-                {
-                    management = new DependencyManagement();
-                }
+                management = new DependencyManagement();

Review comment:
       This is because `management` is always null here?

##########
File path: maven-resolver-transport-wagon/src/main/java/org/eclipse/aether/transport/wagon/WagonTransporter.java
##########
@@ -117,7 +117,7 @@
         wagonRepo.setPermissions( getPermissions( repository.getId(), session ) );
 
         wagonHint = wagonRepo.getProtocol().toLowerCase( Locale.ENGLISH );
-        if ( wagonHint == null || wagonHint.length() <= 0 )
+        if ( wagonHint.length() <= 0 )

Review comment:
       Why not `isEmpty()`?




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