MartinKanters commented on a change in pull request #287: [MNG-6065] - Log fail-level option. URL: https://github.com/apache/maven/pull/287#discussion_r355184759
########## File path: maven-slf4j-provider/src/main/java/org/slf4j/impl/MavenLoggerFactory.java ########## @@ -0,0 +1,82 @@ +package org.slf4j.impl; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.maven.logwrapper.LogLevelRecorder; +import org.apache.maven.logwrapper.MavenSlf4jWrapperFactory; +import org.slf4j.Logger; + +import java.util.Optional; + +/** + * LogFactory for Maven which can create a simple logger or a one which, if set, fails the build on a threshold. + */ +public class MavenLoggerFactory extends SimpleLoggerFactory implements MavenSlf4jWrapperFactory +{ + private LogLevelRecorder logLevelRecorder = null; + + @Override + public void setLogLevelRecorder( LogLevelRecorder logLevelRecorder ) + { + if ( this.logLevelRecorder != null ) + { + throw new IllegalStateException( "Maven logger fail level has already been set." ); + } + + this.logLevelRecorder = logLevelRecorder; + } + + @Override + public Optional<LogLevelRecorder> getLogLevelRecorder() + { + return Optional.ofNullable( logLevelRecorder ); + } + + /** + * Return an appropriate {@link MavenSimpleLogger} instance by name. + */ + @Override + public Logger getLogger( String name ) + { + Logger simpleLogger = loggerMap.get( name ); + if ( simpleLogger != null ) + { + return simpleLogger; + } + else + { + Logger newInstance = getNewLoggingInstance( name ); + Logger oldInstance = loggerMap.putIfAbsent( name, newInstance ); + return oldInstance == null ? newInstance : oldInstance; + } + } + + private Logger getNewLoggingInstance( String name ) Review comment: I ran a small performance test with 3 scenarios. The (current) null-check, a Function/Supplier like Maarten suggested and the removal of any check (the old situation, without any severity logging features). All of them are very quick and the differences are negligible, I guess JIT will also help a lot here. Personally I think we should go for readability/simplicity, so I would keep the null-check. Even though the Function one is quite elegant. ---------------------------------------------------------------- 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] With regards, Apache Git Services
