elharo commented on a change in pull request #48:
URL:
https://github.com/apache/maven-javadoc-plugin/pull/48#discussion_r443127531
##########
File path:
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -6066,23 +6066,15 @@ private URL getResource( final List<String> classPath,
final String resource )
getLog().error( "MalformedURLException: " + e.getMessage() );
}
}
-
- URLClassLoader javadocClassLoader = new URLClassLoader( urls.toArray(
new URL[urls.size()] ), null );
- try
+ URL resourceURL = null;
Review comment:
pull this and the return into the try block
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
##########
@@ -629,36 +650,31 @@ protected static String parseJavadocMemory( String memory
)
throw new IllegalArgumentException( "The memory could not be
null." );
}
- Pattern p = Pattern.compile( "^\\s*(\\d+)\\s*?\\s*$" );
- Matcher m = p.matcher( memory );
+ Matcher m = PARSE_JAVADOC_MEMORY_PATTERN_0.matcher( memory );
if ( m.matches() )
{
return m.group( 1 ) + "m";
}
- p = Pattern.compile( "^\\s*(\\d+)\\s*k(b)?\\s*$",
Pattern.CASE_INSENSITIVE );
- m = p.matcher( memory );
+ m = PARSE_JAVADOC_MEMORY_PATTERN_1.matcher( memory );
Review comment:
in general, we should avoid reusing local variables for different objects
##########
File path:
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
##########
@@ -6066,23 +6066,15 @@ private URL getResource( final List<String> classPath,
final String resource )
getLog().error( "MalformedURLException: " + e.getMessage() );
}
}
-
- URLClassLoader javadocClassLoader = new URLClassLoader( urls.toArray(
new URL[urls.size()] ), null );
- try
+ URL resourceURL = null;
+ try ( URLClassLoader javadocClassLoader = new URLClassLoader(
urls.toArray( new URL[urls.size()] ), null ) )
{
- return javadocClassLoader.getResource( resource );
- }
- finally
+ resourceURL = javadocClassLoader.getResource( resource );
+ } catch ( IOException e )
Review comment:
DANGER: this must be rethrown, not ignored
##########
File path:
src/test/java/org/apache/maven/plugins/javadoc/AggregatorJavadocReportTest.java
##########
@@ -173,20 +173,13 @@ private static String readFile( File file )
{
String strTmp;
Review comment:
could pull this into the try block or even the loop initializer
##########
File path: src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
##########
@@ -1815,7 +1831,7 @@ private static CloseableHttpClient createHttpClient(
Settings settings, URL url
builder.setUserAgent( "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT
5.0)" );
// Some server reject requests that do not have an Accept header
- builder.setDefaultHeaders( Arrays.asList( new BasicHeader(
HttpHeaders.ACCEPT, "*/*" ) ) );
+ builder.setDefaultHeaders( Collections.singletonList( new BasicHeader(
HttpHeaders.ACCEPT, "*/*" ) ) );
Review comment:
stet. Immutability makes Collections.singletonList not semantically
equivalent to Arrays.asList
##########
File path: src/test/java/org/apache/maven/plugins/javadoc/JavadocJarTest.java
##########
@@ -154,7 +154,7 @@ public void testInvalidDestdir()
//check if the javadoc jar file was generated
File generatedFile = new File( getBasedir(),
"target/test/unit/javadocjar-invalid-destdir/target/javadocjar-invalid-destdir-javadoc.jar"
);
- assertTrue( !FileUtils.fileExists( generatedFile.getAbsolutePath() ) );
+ assertFalse(FileUtils.fileExists(generatedFile.getAbsolutePath()));
Review comment:
Maven style uses spaces before ) and after ( unless there are no
arguments
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
