rmannibucau commented on code in PR #1041:
URL: https://github.com/apache/maven/pull/1041#discussion_r1129155652
##########
maven-embedder/src/main/java/org/apache/maven/cli/logging/Slf4jConfigurationFactory.java:
##########
@@ -65,8 +67,14 @@ public static Slf4jConfiguration
getConfiguration(ILoggerFactory loggerFactory)
}
} catch (IOException | ClassNotFoundException | IllegalAccessException
| InstantiationException e) {
e.printStackTrace();
+ hadException = true;
}
- return new UnsupportedSlf4jBindingConfiguration(slf4jBinding,
supported);
+ return new UnsupportedSlf4jBindingConfiguration(
+ slf4jBinding,
+ supported,
Review Comment:
Not really, maven is wrong assuming it knows the entire ecosystem, it is
perfectly valid to use a slf4j binding maven does not know - or if you are
right it means we should drop slf4j from maven because we don't use the
abstraction at the end.
If it is right then we must not issue a warning (an error) when it is an
expected case - the use case triggering this change.
Guess it was more a default implementation than an intent to say a custom
binding is intended.
Assuming any custom binding will bring a custom maven integration is also a
high assumption for the little usage we do of that integration (a single CLI
toggle which is rarely used in practise) so hope we don't go that path which
would be the alternative.
--
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]
