kriegaex commented on code in PR #854:
URL:
https://github.com/apache/maven-shade-plugin/pull/854#discussion_r4059392237
##########
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##########
@@ -191,12 +191,11 @@ private boolean removeServicesFromDir(Clazzpath cp,
Set<Clazz> neededClasses, St
continue;
}
- try (BufferedReader configFileReader =
- new BufferedReader(new InputStreamReader(new
FileInputStream(serviceProviderConfigFile), UTF_8))) {
+ try (BufferedReader configFileReader =
Files.newBufferedReader(serviceProviderConfigFile.toPath(), UTF_8)) {
// check whether the found classes use services in turn
repeatScan |= scanServiceProviderConfigFile(cp,
configFileReader);
} catch (final IOException e) {
- log.warn(e.getMessage());
+ log.warn("Failed to scan service provider config file " +
serviceProviderConfigFile, e);
Review Comment:
I would prefer
```java
log.warn( "Failed to scan service provider config file " +
serviceProviderConfigFile );
log.debug( e );
```
Less noise in the `warn` log, full exception stack trace in the `debug` log.
##########
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##########
@@ -222,11 +221,11 @@ private boolean removeServicesFromJar(Clazzpath cp,
Set<Clazz> neededClasses, St
// check whether the found classes use services in turn
repeatScan = scanServiceProviderConfigFile(cp,
configFileReader);
} catch (final IOException e) {
- log.warn(e.getMessage());
+ log.warn("Failed to scan JAR entry " + jarEntry.getName(),
e);
Review Comment:
I would prefer
```java
log.warn( "Failed to scan JAR entry " + jarEntry + " in jar " +
jar.getName() );
log.debug( e );
```
This also mentions the JAR name. Moreover, there is less noise in the `warn`
log, but the full exception stack trace in the `debug` log.
##########
src/main/java/org/apache/maven/plugins/shade/filter/MinijarFilter.java:
##########
@@ -222,11 +221,11 @@ private boolean removeServicesFromJar(Clazzpath cp,
Set<Clazz> neededClasses, St
// check whether the found classes use services in turn
repeatScan = scanServiceProviderConfigFile(cp,
configFileReader);
} catch (final IOException e) {
- log.warn(e.getMessage());
+ log.warn("Failed to scan JAR entry " + jarEntry.getName(),
e);
}
}
} catch (final IOException e) {
- log.warn("Not a JAR file candidate. Ignoring classpath element '"
+ fileName + "' (" + e + ").");
+ log.warn("Not a JAR file candidate. Ignoring classpath element '"
+ fileName + "'", e);
Review Comment:
Here, I also would rather split the warning and logging the exception stack
trace, see above.
--
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]
