[
https://issues.apache.org/jira/browse/MNG-7866?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17764709#comment-17764709
]
ASF GitHub Bot commented on MNG-7866:
-------------------------------------
sebastien-doyon commented on code in PR #1220:
URL: https://github.com/apache/maven/pull/1220#discussion_r1324599510
##########
maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java:
##########
@@ -152,16 +149,15 @@ private ClassRealm createRealm(
List<String> parentImports,
Map<String, ClassLoader> foreignImports,
List<Artifact> artifacts) {
- Set<String> artifactIds = new LinkedHashSet<>();
-
- List<ClassRealmConstituent> constituents = new ArrayList<>();
+ List<ClassRealmConstituent> constituents = new ArrayList<>(artifacts
== null ? 0 : artifacts.size());
- if (artifacts != null) {
+ if (artifacts != null && !artifacts.isEmpty()) {
for (Artifact artifact : artifacts) {
if (!isProvidedArtifact(artifact)) {
- artifactIds.add(getId(artifact));
if (artifact.getFile() != null) {
constituents.add(new
ArtifactClassRealmConstituent(artifact));
+ } else if (logger.isDebugEnabled()) {
+ logger.debug(" Excluded: {}", getId(artifact));
Review Comment:
I think you are right, but to be sure all path that are not adding the
artifact to the constituents list are logged as excluded, I can keep the
condition here and add the same else block to the "if isProvidedArtifact" has
you proposed. What do you think? Alternative fix could be a simple if after
the"if isProvidedArtifact" if I add a continue; after the line 158. This is a
matter of taste, what do you think?
> Improvements to the logging API usage (technical debt)
> ------------------------------------------------------
>
> Key: MNG-7866
> URL: https://issues.apache.org/jira/browse/MNG-7866
> Project: Maven
> Issue Type: Improvement
> Components: Core
> Affects Versions: 4.0.0-alpha-2
> Reporter: sebastien
> Priority: Minor
> Fix For: 4.0.0-alpha-8
>
>
> Since maven 4 is now using the Slf4J logger API, some logging code can be
> improved.
> Typical improvements are:
> * Use message formats with placeholders to avoid premature formatting and
> avoid the unnecessary garbage when then log level is disabled. Example :
>
> {code:java}
> logger.debug("Toolchains configuration was not found at " +
> userToolchainsFile);
> {code}
> can be replaced with :
> {code:java}
> logger.debug("Toolchains configuration was not found at {}",
> userToolchainsFile);{code}
> * Guarding some logging statements with conditionals on isXXXXEnabled() to
> avoid unnecessary garbage when then log level is disabled. Useful when some
> formatting must be done outside the logger call. Example :
>
> {code:java}
> } else {
> Lifecycle original = phaseToLifecycleMap.get(phase);
> logger.warn("Duplicated lifecycle phase " + phase + ".
> Defined in " + original.getId()
> + " but also in " + lifecycle.getId());
> }
> {code}
> can be replaced with the following code to avoid the cost of the map lookup :
>
> {code:java}
> } else if (logger.isWarnEnabled()) {
> Lifecycle original = phaseToLifecycleMap.get(phase);
> logger.warn(
> "Duplicated lifecycle phase {}. Defined in {} but
> also in {}",
> phase,
> original.getId(),
> lifecycle.getId());
> }
> {code}
> * Remove some unneeded conditional guarding to avoid testing twice if the
> log level is enabled, like for example :
>
> {code:java}
> if (logger.isDebugEnabled()) {
> logger.debug("Lifecycle " + lifecycle);
> }
> {code}
> can be replaced with :
> {code:java}
> logger.debug("Lifecycle {}", lifecycle);{code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
