search

emorynlp / nlp4j-old

NLP tools developed by Emory University.
Other
60 stars 19 forks source link

slf4j would be more flexible than raw log4j #30

Open benson-basis opened 8 years ago

benson-basis commented 8 years ago

Many of us use SLF4J as the API to logging rather than going directly to log4j. If you are amenable, I'll provide PRs to the repos.

jdchoi77 commented 8 years ago

I believe that change had been made to the latest version (1.1.2):

https://github.com/emorynlp/nlp4j-common/blob/master/pom.xml

I'm closing this issue, but please let me know if it's not what you asked for. Thanks.

benson-basis commented 8 years ago

I'd like to get rid of the single logger in BinUtils and have per-class loggers, as is the usual practice. I'd like to make sure that slf4j-log4j is only in the tree as test dependency, so that people who want to use another logger back end can do so. I'd like to take all the '\n''s off the log messages, and put it in the log4j.properties.

This would be a chain of PR's starting at the bottom, of course. What do you think?

I'd sort of like to propose merging the many repos into one with multiple maven modules: do you really need to release them independently.

On Thu, Jul 21, 2016 at 2:25 PM, Jinho D. Choi notifications@github.com wrote:

I believe that change had been made to the latest version (1.1.2):

https://github.com/emorynlp/nlp4j-common/blob/master/pom.xml

I'm closing this issue, but please let me know if it's not what you asked for. Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/emorynlp/nlp4j/issues/30#issuecomment-234340748, or mute the thread https://github.com/notifications/unsubscribe-auth/ADM9z53F78M6M1jBTgViKFynIkMC4MOdks5qX7mFgaJpZM4JLga2 .

jdchoi77 commented 8 years ago

For SLF4J, yes that is a much better practice; I just haven't found time to do it. For many repos, it has some history; originally, I had everything setup in one repository (it was before maven), and some people didn't want it to be in one repo, so I broke them down into what we have now, but now we use maven and it offers the modulation, we could merge them back and create multiple maven models. Would you be interested in supporting this? Thanks!

benson-basis commented 8 years ago

I would be happy to do both of merging and slf4j-ing. I think it would make sense to do merging first. If that makes sense to you, I'll make a new repo in my space, do the git/maven manipulation, and then you can clone it into your org. Unless you want to add me to the org.

On Thu, Jul 21, 2016 at 2:55 PM, Jinho D. Choi notifications@github.com wrote:

For SLF4J, yes that is a much better practice; I just haven't found time to do it. For many repos, it has some history; originally, I had everything setup in one repository (it was before maven), and some people didn't want it to be in one repo, so I broke them down into what we have now, but now we use maven and it offers the modulation, we could merge them back and create multiple maven models. Would you be interested in supporting this? Thanks!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/emorynlp/nlp4j/issues/30#issuecomment-234349368, or mute the thread https://github.com/notifications/unsubscribe-auth/ADM9z_HMPR0S75xPSkFa03dd1ZpbdD16ks5qX8CJgaJpZM4JLga2 .

jdchoi77 commented 8 years ago

I just created a repository and add you as a collaborator. Could you please use this repository? If you'd prefer to use your own, I'll be fine with that too. Thanks.

benson-basis commented 8 years ago

Using yours is great.

On Jul 21, 2016 3:13 PM, "Jinho D. Choi" notifications@github.com wrote:

I just created a repository and add you as a collaborator. Could you please use this repository? If you'd prefer to use your own, I'll be fine with that too. Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/emorynlp/nlp4j/issues/30#issuecomment-234354316, or mute the thread https://github.com/notifications/unsubscribe-auth/ADM9z-Tz_s5OfdajakI9wbjL5-MX0SIDks5qX8TogaJpZM4JLga2 .