[ 
https://issues.apache.org/jira/browse/IGNITE-788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14597815#comment-14597815
 ] 

Artem Shutak commented on IGNITE-788:
-------------------------------------

Hi, Gianfranco!

I see my comments:
1. In second time, could you add IDE specific files to .gitignire? See .project.
2. Ok, you've fixed Log4jFileAware name. What about the rest of files?. See 
Log4JLogger and etc.
3. Code style. 
- Log4JLogger line 92. It should be 2 "*/" instead of "* */". See all places.
- All classes and methods have to have javadoc. 
- Parameter description at javadoc should start on the same line as paramether 
(Log4JLogger line 161).
- remove empty lines at first line of class and between methods and javadoc 
(Log4JLogger 187).
4. Why did you change something under ignite-examples?
5. Log4JLogger have bad javadoc. It will confuse users.
6. Tests.
- Again, we are using JUnit 3 (not older versions). So, you cannot use Test 
annotation.
- Need to fix packaging of test classes (see log4j module).

My main point, please, do all in the same way as it is done at log4j. Do not 
reinvent the wheel ;)

Thanks,
Artem.

> add support for log4j2 into org.apache.ignite.logger.log4j package 
> (Log4J2Logger)
> ---------------------------------------------------------------------------------
>
>                 Key: IGNITE-788
>                 URL: https://issues.apache.org/jira/browse/IGNITE-788
>             Project: Ignite
>          Issue Type: New Feature
>          Components: general, newbie
>    Affects Versions: sprint-3
>            Reporter: Yakov Zhdanov
>            Assignee: Gianfranco Murador
>              Labels: newbie
>             Fix For: sprint-7
>
>         Attachments: ignite-sprint-5_ignite-788.patch, patch-ingnite-788.zip
>
>
> This should be a new module. See {{ignite-log4j}} module for reference



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to