On May 13, 2009, at 6:01 PM, Vincent Massol wrote:
>
> On May 13, 2009, at 5:53 PM, Sergiu Dumitriu wrote:
>
>> Vincent Massol wrote:
>>> On May 13, 2009, at 5:43 PM, Sergiu Dumitriu wrote:
>>>
>>>> Vincent Massol wrote:
>>>>> On May 13, 2009, at 5:30 PM, Sergiu Dumitriu wrote:
>>>>>
>>>>>> Vincent Massol wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I wanted to see if we could move our LogEnabled lifecycle phase
>>>>>>> to a
>>>>>>> Logging component. I think it's not going to work since this
>>>>>>> means
>>>>>>> injecting a LoggingFactory/LoggingManager component and you
>>>>>>> need to
>>>>>>> call getLogger(this.getClass()) to get access to the Logger
>>>>>>> which
>>>>>>> is
>>>>>>> awkward.
>>>>>>>
>>>>>>> What I propose:
>>>>>>>
>>>>>>> 1) Use SLF4J (drop the slf4j-log4j jar in our WEB-INF/lib so
>>>>>>> that
>>>>>>> SLF4J uses log4j by default)
>>>>>>> 2) Drop the JCL/JUL/LOG4j SLF4J legacy jars in our WEB-INF/lib
>>>>>>> too
>>>>>>> and exclude the JCL/JUL/LOG4J jars from our poms so that all
>>>>>>> third
>>>>>>> party logs go to our logging system
>>>>>>> 3) Non component code should use a SLF4J's LoggerFactory
>>>>>>> directly
>>>>>>>
>>>>>>> 4a) Keep LogEnabled and AbstractLogEnabled for our components
>>>>>>> or
>>>>>>> 4b) Automatically inject a Logger and a ComponentManager when
>>>>>>> there
>>>>>>> are fields with these types in a component class.
>>>>>>>
>>>>>>> I like 4b) for its simplicity but I'm worried by the "magical"
>>>>>>> aspect
>>>>>>> of it.
>>>>>> But... Why do we need 4 at all?
>>>>> You mean use a static and don't do IOC?
>>>>>
>>>>> I don't like it it has all the problems of static.
>>>>>
>>>> Why not just have a plain field, like:
>>>>
>>>> final Logger logger = LoggerFactory.getLogger(Wombat.class);
>>>>
>>>> Is the logger a component? It's just plain logging, we don't need
>>>> to
>>>> go
>>>> that deep. IOC is fine where it's useful, but here it's just
>>>> overhead IMO.
>>>
>>> Funny you say this when I find this in your code:
>>>
>>> // TODO It would be better to use a custom logger class, how
>>> to do that?
>>> StringOutputStream log = new StringOutputStream();
>>> PrintStream out = System.out;
>>> System.setOut(new PrintStream(log));
>>> engine.evaluate(context, writer, "mytemplate",
>>> new StringReader("#set($foo = $date.getYear())$foo
>>> $date.month"));
>>> System.setOut(out);
>>>
>>> :)
>>>
>>> Your example makes it hard to unit test. I'd personally see it as a
>>> regression to what we currently have.
>>
>> In the meantime I found the answer for this question, and it's
>> simply to
>> use a different log4j.properties file for tests (src/test/
>> resources/).
>
> That's not very nice and wrong IMO since it creates a strong
> dependency with our logging system and log4j when our system is
> independent of the logging system chosen.
A mock Logger with an expectation would be much better.
-Vincent
_______________________________________________
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
