(I'm talking about doing JUnit tests on the factory methods in case that
was not clear.)
On Mon, Jun 16, 2014 at 9:42 PM, Remko Popma <remko.po...@gmail.com> wrote:
> ...and another (shorter) way would be to add a comment for each parameter:
> factoryMethod(
> null, // loggerName
> null, // Marker
> null, // LoggerFQCN
> null, // Level
> null, // Message
> null, // Throwable
> null, // ContextMap
> null, // ContextStack
> null, // ThreadName
> null, // Location
> 0); // CurrentTime
>
>
>
> On Mon, Jun 16, 2014 at 9:36 PM, Remko Popma <remko.po...@gmail.com>
> wrote:
>
>> One very simple way I use to clarify which parameter is what is to
>> declare a local variable for each param.
>> For example:
>>
>> (and this is not strictly a @Plugin factory method, but you get my
>> point...)
>>
>> public class RingBufferLogEventTest {
>> @Test
>> public void testGetLevelReturnsOffIfNullLevelSet() {
>> RingBufferLogEvent evt = new RingBufferLogEvent();
>> String loggerName = null;
>> Marker marker = null;
>> String fqcn = null;
>> Level level = null;
>> Message data = null;
>> Throwable t = null;
>> Map<String, String> map = null;
>> ContextStack contextStack = null;
>> String threadName = null;
>> StackTraceElement location = null;
>> long currentTimeMillis = 0;
>> evt.setValues(null, loggerName, marker, fqcn, level, data, t, map,
>> contextStack, threadName, location, currentTimeMillis);
>> assertEquals(Level.OFF, evt.getLevel());
>> }
>>
>> To me this is just as clear as
>>
>> builder.setLoggerName(null)
>> .withMarker(null)
>> .withLoggerFQCN(null)
>> .withLevel(null)
>> .withMessage(null)
>> .withThrowable(null)
>> .withContextMap(null)
>> .withContextStack(null)
>> .withThreadName(null)
>> .withLocation(null)
>> .withCurrentTime(0)
>> .build()
>>
>>
>> On Mon, Jun 16, 2014 at 1:10 PM, Matt Sicker <boa...@gmail.com> wrote:
>>
>>> Well, I can live with factory methods if we have a better method for
>>> doing unit tests. Not all unit tests need an XML configuration file.
>>>
>>>
>>> On 15 June 2014 22:04, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>>>
>>>> Matt,
>>>>
>>>> The only objection I have to builders is that there should only be one
>>>> way to configure plugins and I have neither the time or energy to convert
>>>> all plugins from factories to builders. With 130+ open issues I think our
>>>> time is better focused there instead of fixing something that already
>>>> works.
>>>>
>>>> And, FWIW, the only place you will see createAppender coded like that
>>>> is in unit tests and there are a bunch of ways to make that clearer.
>>>>
>>>> Ralph
>>>>
>>>> On Jun 15, 2014, at 7:19 PM, Matt Sicker <boa...@gmail.com> wrote:
>>>>
>>>> I'm really against using factory methods due to language limitations in
>>>> Java. You can't specify default values, for one. Two, the more parameters a
>>>> factory takes, the crazier the method is. Seriously, tell me what this
>>>> method is specifying:
>>>>
>>>> FileAppender.createAppender("true", "true", "true", "true", "true",
>>>> "true", "true", "4096", null, null, "false", null, null);
>>>>
>>>>
>>>> On 15 June 2014 21:05, Remko Popma <remko.po...@gmail.com> wrote:
>>>>
>>>>> I'm fine with just the factory methods too.
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On 2014/06/16, at 9:44, Scott Deboy <scott.de...@gmail.com> wrote:
>>>>>
>>>>> +1
>>>>> On Jun 15, 2014 4:05 PM, "Ralph Goers" <ralph.go...@dslextreme.com>
>>>>> wrote:
>>>>>
>>>>>> Do we need the builders? As I said, I prefer only one way for
>>>>>> creating plugins.
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>> On Jun 15, 2014, at 2:49 PM, Remko Popma <remko.po...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>> I see. I agree that the original format is much nicer.
>>>>>>
>>>>>> Matt, do you think you can achieve this with the builders?
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On 2014/06/16, at 1:29, Ralph Goers <ralph.go...@dslextreme.com>
>>>>>> wrote:
>>>>>>
>>>>>> While you improved some of the existing messages, you really didm’t
>>>>>> address what I wanted fixed. The previous debug logs would have had
>>>>>> messages similar to:
>>>>>>
>>>>>> Calling createLayout on class
>>>>>> org.apache.logging.log4j.core.layout.PatternLayout for element
>>>>>> PatternLayout with params(pattern="%d{HH:mm:ss.SSS} [%t] %-5level
>>>>>> %logger{36} - %msg%n",
>>>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>>>> null, charset="null", alwaysWriteExceptions="null")
>>>>>>
>>>>>> Calling createAppender on class
>>>>>> org.apache.logging.log4j.core.appender.ConsoleAppender for element
>>>>>> Console
>>>>>> with params(PatternLayout(%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} -
>>>>>> %msg%n), null, target="SYSTEM_OUT", name="console", follow="null",
>>>>>> ignoreExceptions="null")
>>>>>>
>>>>>> Calling createAppenderRef on class
>>>>>> org.apache.logging.log4j.core.config.AppenderRef for element appender-ref
>>>>>> with params(ref="console", level="null", null)
>>>>>> 2013-09-20 15:06:55,749 DEBUG Calling createLogger on class
>>>>>> org.apache.logging.log4j.core.config.LoggerConfig$RootLogger for element
>>>>>> root with params(additivity="null", level="error",
>>>>>> includeLocation="null",
>>>>>> AppenderRef={console}, Properties={},
>>>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>>>> null)
>>>>>>
>>>>>> The current log emits stuff like:
>>>>>>
>>>>>> 2014-06-15 09:07:19,432 DEBUG Building Plugin[name=layout,
>>>>>> class=org.apache.logging.log4j.core.layout.XmlLayout]. Searching for
>>>>>> builder factory method...
>>>>>> 2014-06-15 09:07:19,435 DEBUG No builder factory method found in
>>>>>> class org.apache.logging.log4j.core.layout.XmlLayout.
>>>>>> 2014-06-15 09:07:19,435 DEBUG Still building Plugin[name=layout,
>>>>>> class=org.apache.logging.log4j.core.layout.XmlLayout]. Searching for
>>>>>> factory method...
>>>>>> 2014-06-15 09:07:19,436 DEBUG Found factory method [createLayout]:
>>>>>> public static org.apache.logging.log4j.core.layout.XmlLayout
>>>>>> org.apache.logging.log4j.core.layout.XmlLayout.createLayout(boolean,boolean,boolean,boolean,java.nio.charset.Charset).
>>>>>> 2014-06-15 09:07:19,436 DEBUG Generating parameters for factory
>>>>>> method [createLayout]...
>>>>>> 2014-06-15 09:07:19,456 DEBUG Attribute(locationInfo="false") - no
>>>>>> value specified, using default.
>>>>>> 2014-06-15 09:07:19,456 DEBUG Attribute(properties="false") - no
>>>>>> value specified, using default.
>>>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(complete="true") - no value
>>>>>> specified, using default.
>>>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(compact="false") - no value
>>>>>> specified, using default.
>>>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(charset="UTF-8") - no value
>>>>>> specified, using default.
>>>>>> 2014-06-15 09:07:19,587 DEBUG Built Plugin[name=layout] OK from
>>>>>> factory method.
>>>>>> 2014-06-15 09:07:19,588 DEBUG Building Plugin[name=appender,
>>>>>> class=org.apache.logging.log4j.core.appender.FileAppender]. Searching for
>>>>>> builder factory method...
>>>>>> 2014-06-15 09:07:19,588 DEBUG No builder factory method found in
>>>>>> class org.apache.logging.log4j.core.appender.FileAppender.
>>>>>> 2014-06-15 09:07:19,589 DEBUG Still building Plugin[name=appender,
>>>>>> class=org.apache.logging.log4j.core.appender.FileAppender]. Searching for
>>>>>> factory method...
>>>>>> 2014-06-15 09:07:19,589 DEBUG Found factory method [createAppender]:
>>>>>> public static org.apache.logging.log4j.core.appender.FileAppender
>>>>>> org.apache.logging.log4j.core.appender.FileAppender.createAppender(java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,org.apache.logging.log4j.core.Layout,org.apache.logging.log4j.core.Filter,java.lang.String,java.lang.String,org.apache.logging.log4j.core.config.Configuration).
>>>>>> 2014-06-15 09:07:19,589 DEBUG Generating parameters for factory
>>>>>> method [createAppender]...
>>>>>> 2014-06-15 09:07:19,595 DEBUG
>>>>>> Attribute(fileName="target/XmlCompleteFileAppenderTest.log")
>>>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(append="false")
>>>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(locking="null")
>>>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(name="XmlFile")
>>>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(immediateFlush="false")
>>>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(ignoreExceptions="null")
>>>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(bufferedIo="null")
>>>>>> 2014-06-15 09:07:19,598 DEBUG Attribute(bufferSize="null")
>>>>>> 2014-06-15 09:07:19,598 DEBUG
>>>>>> XMLLayout(org.apache.logging.log4j.core.layout.XmlLayout@5eef9f84)
>>>>>> 2014-06-15 09:07:19,598 DEBUG Attribute(advertise="null")
>>>>>> 2014-06-15 09:07:19,599 DEBUG Attribute(advertiseUri="null")
>>>>>> 2014-06-15 09:07:19,599 DEBUG
>>>>>> Configuration(/Users/rgoers/projects/apache/logging/log4j/log4j2/trunk/log4j-core/target/test-classes/XmlCompleteFileAppenderTest.xml)
>>>>>> 2014-06-15 09:07:19,601 DEBUG Starting FileManager
>>>>>> target/XmlCompleteFileAppenderTest.log
>>>>>>
>>>>>> The previous format was a lot more compact as it essentially showed
>>>>>> you the parameters being passed to the factory method in one line while
>>>>>> identifying the class it came from and the configuration element that
>>>>>> triggered it. The new log emits that info as individual lines with a few
>>>>>> messages that are just noise.
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Jun 14, 2014, at 10:11 PM, Remko Popma <remko.po...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>> I've done some work on this, there may be more places to improve, I
>>>>>> mainly focused on PluginBuilder and PluginAttributeVisitor.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Jun 12, 2014 at 2:09 AM, Matt Sicker <boa...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Yeah, I liked the prettier logging format. I was planning to add it
>>>>>>> back in, but it appears as though I completely forgot about it! The
>>>>>>> "new"
>>>>>>> format was a quick placeholder. I'll try and work on that this week.
>>>>>>>
>>>>>>>
>>>>>>> On 10 June 2014 19:47, Gary Gregory <garydgreg...@gmail.com> wrote:
>>>>>>>
>>>>>>>> Maybe Matt can shed a light on this?
>>>>>>>>
>>>>>>>> Gary
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Jun 10, 2014 at 8:43 PM, Ralph Goers <rgo...@apache.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> I don't know exactly what I would be vetoing. I have no problem
>>>>>>>>> with some of the refactoring. The commit(s) that changed the logging
>>>>>>>>> probably happened weeks ago and I am just noticing now.
>>>>>>>>>
>>>>>>>>> But yes, I want the logging aspect of the changes reverted back to
>>>>>>>>> what was previously being done.
>>>>>>>>>
>>>>>>>>> Sent from my iPad
>>>>>>>>>
>>>>>>>>> On Jun 10, 2014, at 5:34 PM, Gary Gregory <garydgreg...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Well, for Log4j Plugins, one way to configure should be plenty. I
>>>>>>>>> am OK with the factory method pattern, while it makes for some long
>>>>>>>>> signatures, I like that it is all in one place.
>>>>>>>>>
>>>>>>>>> May I suggest a simple "-1" reply on the ML on the changes to
>>>>>>>>> logging? Do you feel a VETO is inappropriate here?
>>>>>>>>>
>>>>>>>>> Gary
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Jun 10, 2014 at 7:57 PM, Ralph Goers <rgo...@apache.org>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> I think the discussion was not on its own thread. I thought
>>>>>>>>>> there was agreement that there should be only one way to configure
>>>>>>>>>> plugins.
>>>>>>>>>> I prefer the factory method simply because it would be a whole lot
>>>>>>>>>> of
>>>>>>>>>> effort to convert everything to a builder and I just don't think the
>>>>>>>>>> benefit is worth the effort.
>>>>>>>>>>
>>>>>>>>>> I spent a lot of time making the debug output "nice" and easily
>>>>>>>>>> understandable so I am a bit upset that it was tossed and replaced
>>>>>>>>>> with
>>>>>>>>>> what you see below.
>>>>>>>>>>
>>>>>>>>>> Ralph
>>>>>>>>>>
>>>>>>>>>> Sent from my iPad
>>>>>>>>>>
>>>>>>>>>> On Jun 10, 2014, at 4:31 PM, Gary Gregory <garydgreg...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> On Tue, Jun 10, 2014 at 7:11 PM, Ralph Goers <
>>>>>>>>>> ralph.go...@dslextreme.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> I am working on a new Appender and am noticing that the debug
>>>>>>>>>>> output is now far less useful than it used to be. I used to see the
>>>>>>>>>>> factory
>>>>>>>>>>> method being invoked with all of its parameters very nicely
>>>>>>>>>>> formatted. Now
>>>>>>>>>>> I see
>>>>>>>>>>>
>>>>>>>>>>> 2014-06-10 16:02:37,858 DEBUG No compatible method annotated
>>>>>>>>>>> with interface
>>>>>>>>>>> org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory
>>>>>>>>>>> found in
>>>>>>>>>>> class class org.apache.logging.log4j.web.appender.ServletAppender.
>>>>>>>>>>> 2014-06-10 16:02:37,858 DEBUG Found factory method class
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender.public static
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender.createAppender(org.apache.logging.log4j.core.Layout,org.apache.logging.log4j.core.Filter,java.lang.String,java.lang.String).
>>>>>>>>>>> 2014-06-10 16:02:37,864 DEBUG Constructing plugin of type class
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>>>>>> 2014-06-10 16:02:37,864 DEBUG PatternLayout(%m%n)
>>>>>>>>>>> 2014-06-10 16:02:37,864 DEBUG Constructing plugin of type class
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>>>>>> 2014-06-10 16:02:37,865 DEBUG Constructing plugin of type class
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>>>>>> 2014-06-10 16:02:37,865 DEBUG Attribute(name="Servlet")
>>>>>>>>>>> 2014-06-10 16:02:37,865 DEBUG Constructing plugin of type class
>>>>>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>>>>>> 2014-06-10 16:02:37,865 DEBUG Null string given to convert.
>>>>>>>>>>> Using default [null].
>>>>>>>>>>> 2014-06-10 16:02:37,866 DEBUG Attribute(ignoreExceptions="null")
>>>>>>>>>>>
>>>>>>>>>>> This is far more verbose, repetitive, and is nowhere near as
>>>>>>>>>>> clear as it used to be.
>>>>>>>>>>>
>>>>>>>>>>> Can you please get the logging output back to the old format?
>>>>>>>>>>>
>>>>>>>>>>> Also, can we change PatternLayout back to a factory and remove
>>>>>>>>>>> the message about no builder factory being present?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think we need to decide how many ways there are to configure a
>>>>>>>>>> plugin: factory, builder, and whatever else we've been discussing.
>>>>>>>>>> This is
>>>>>>>>>> getting quite confusing!
>>>>>>>>>>
>>>>>>>>>> I thought we had a thread going on the topic already...
>>>>>>>>>>
>>>>>>>>>> Gary
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Ralph
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>>>>> <http://www.manning.com/bauer3/>
>>>>>>>>>> JUnit in Action, Second Edition
>>>>>>>>>> <http://www.manning.com/tahchiev/>
>>>>>>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>>>>> Home: http://garygregory.com/
>>>>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>>>> <http://www.manning.com/bauer3/>
>>>>>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>>>> Home: http://garygregory.com/
>>>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>>> <http://www.manning.com/bauer3/>
>>>>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>>> Home: http://garygregory.com/
>>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <boa...@gmail.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <boa...@gmail.com>
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <boa...@gmail.com>
>>>
>>
>>
>
