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.

-Vincent

_______________________________________________
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to