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>