[GitHub] [maven-war-plugin] bmarwell commented on a diff in pull request #23: [MWAR-453] Require Java 8

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

bmarwell commented on code in PR #23:
URL: https://github.com/apache/maven-war-plugin/pull/23#discussion_r875796588


##########
src/test/java/org/apache/maven/plugins/war/AbstractWarMojoTest.java:
##########
@@ -74,7 +74,7 @@ protected void configureMojo( AbstractWarMojo mojo, List<String> filters, File c
             new DefaultMavenExecutionRequest().setSystemProperties( System.getProperties() ).setStartTime( new Date() );
 
         MavenSession mavenSession =
-            new MavenSession( (PlexusContainer) null, (RepositorySystemSession) null, request, null );
+            new MavenSession( null, null, request, null );

Review Comment:
   Are you sure this is a good idea? Imagine a new constructor came  up with four other parameters.



##########
src/test/java/org/apache/maven/plugins/war/WarOverlaysTest.java:
##########
@@ -149,7 +149,7 @@ public void testDefaultOverlays()
             assertOverlayedFile( webAppDirectory, "overlay-two", "admin.jsp" );
 
             // Ok now check that there is no more files/directories
-            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] { MANIFEST_PATH } );
+            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] {MANIFEST_PATH} );

Review Comment:
   That is not a Java 8 change.



##########
src/test/java/org/apache/maven/plugins/war/WarOverlaysTest.java:
##########
@@ -111,7 +111,7 @@ public void testDefaultOverlay()
             assertOverlayedFile( webAppDirectory, "overlay-one", "login.jsp" );
 
             // Ok now check that there is no more files/directories
-            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] { MANIFEST_PATH } );
+            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] {MANIFEST_PATH} );

Review Comment:
   That is not a Java 8 change.



##########
src/test/java/org/apache/maven/plugins/war/WarOverlaysTest.java:
##########
@@ -362,7 +362,7 @@ public void testOverlaysIncludesExcludesWithMultipleDefinitions()
             assertOverlayedFile( webAppDirectory, "overlay-full-2", "WEB-INF/lib/c.jar" );
 
             // Ok now check that there is no more files/directories
-            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] { MANIFEST_PATH } );
+            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] {MANIFEST_PATH} );

Review Comment:
   That is not a Java 8 change.



##########
src/test/java/org/apache/maven/plugins/war/WarOverlaysTest.java:
##########
@@ -432,7 +432,7 @@ public void testOverlaysIncludesExcludesWithMultipleDefinitions2()
             assertOverlayedFile( webAppDirectory, "overlay-full-2", "WEB-INF/lib/c.jar" );
 
             // Ok now check that there is no more files/directories
-            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] { MANIFEST_PATH } );
+            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] {MANIFEST_PATH} );

Review Comment:
   That is not a Java 8 change.



##########
src/test/java/org/apache/maven/plugins/war/WarOverlaysTest.java:
##########
@@ -293,7 +293,7 @@ private void assertScenariOne( String testId, File webAppDirectory )
             assertOverlayedFile( webAppDirectory, "overlay-full-1", "WEB-INF/lib/c.jar" );
 
             // Ok now check that there is no more files/directories
-            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] { MANIFEST_PATH } );
+            final FileFilter filter = new FileFilterImpl( webAppDirectory, new String[] {MANIFEST_PATH} );

Review Comment:
   That is not a Java 8 change.



##########
src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java:
##########
@@ -342,14 +344,16 @@ protected boolean copyFile( WarPackagingContext context, File source, File desti
     {
         context.addResource( targetFilename );
 
-        if ( onlyIfModified && destination.lastModified() >= source.lastModified() )
+        BasicFileAttributes readAttributes = Files.readAttributes( source.toPath(), BasicFileAttributes.class );
+        if ( onlyIfModified && destination.lastModified() >= readAttributes.lastModifiedTime()
+                .toMillis() )

Review Comment:
   What is the advantage to use another API here?



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