Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slf4j #3

Merged
merged 22 commits into from
Jul 3, 2012
Merged

Slf4j #3

merged 22 commits into from
Jul 3, 2012

Conversation

hazendaz
Copy link
Member

Here is commons-logging switched to slf4j. I updated the classpath that was missed in the last revision (sorry about that). My change log is slightly different than what you went with. I retained what I originally put in there (ie 1.5 snapshot) and merged in changes you made to show users updating it (ie @hazendaz). All tests ran successfully with exception to the four tests I previously mentioned failing on my machine when I did the guava/junit update.

One note here: Spring still uses commons-logging. There is some chatter on their site about upgrading but that seems to have gone nowhere. So one additional jar included with the slf4j package and now on the classpath is for jcl to slf4j. This allows spring to work as is without further changes. Users that might use slf4j can use additional jar to point to the implementations they need such as log4j by using the slf4j to log4j jar. Many others are included. I provided sample xml files in the demos for logback implementation as it is native slf4j solution but it isn't necessary if any of the others are used.

I think this gives everyone lots of additional logging ability. This also provides smarter logging to this application moving forwards if needed.

Added jars for slf4j with jars to allow jcl, jdk, log4j, nop, simple
logging solutions.  Also added logback for native implementation.
Added sample logback.xml to all demos allowing others to easy set this
up for testing.  Logback is not required.  It is just one of many
solutions (native in this case) for slf4j.
Replaced logging with slf4j logging.  As with commons-logging, any
logger can be utilized.  Jars are included.  Also have logback for
native implementation which requires only slf4j-api
Spring still uses commons logging.  In order to switch over to slf4j,
needed jcl-over-slf4j jar.  This was found in unit testing as tests
failed with commons-logging not found.  Adding this fixed issue allowing
tests to complete.
@dblock
Copy link
Collaborator

dblock commented Jul 1, 2012

This is pretty simple, and I spent a couple of hours reading about this. I am sold. There're a few issues.

  • You need to merge from upstream please to make this merge clean. It's all in the changelog. I would make it look like this:
* [#3](https://github.com/dblock/waffle/pull/3): Converted commons-logging to slf4j logging - [@hazendaz](https://github.com/hazendaz).
  * Added slf4j with additional jars for jcl, jdk, log4j, nop, simple logging solutions.
  * Added jcl over slf4j jar for spring as it still uses commons-logging.
  * Added logback for native slf4j solution.
  * Added sample xml files for logback solution to all demos.
* [#2](https://github.com/dblock/waffle/pull/2): Upgraded thirdparty Guava to 12.0 - [@hazendaz](https://github.com/hazendaz).
* [#2](https://github.com/dblock/waffle/pull/2): Upgraded thirdparty JUnit to 4.10 - [@hazendaz](https://github.com/hazendaz).
* [#1](https://github.com/dblock/waffle/pull/1): Adjusted logging from info to debug to reduce noise level - [@mcfly83](https://github.com/mcfly83).
  • ANT build is broken, run ant from the command line. The first part was easy, need to replace commons-logging by slf4j, but there're tests failing next with this:
N/A

java.lang.ExceptionInInitializerError
at org.slf4j.impl.StaticLoggerBinder.<init>(StaticLoggerBinder.java:73)
at org.slf4j.impl.StaticLoggerBinder.<clinit>(StaticLoggerBinder.java:42)
at org.slf4j.LoggerFactory.bind(LoggerFactory.java:128)
at org.slf4j.LoggerFactory.performInitialization(LoggerFactory.java:107)
at org.slf4j.LoggerFactory.getILoggerFactory(LoggerFactory.java:295)
at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:269)
at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:281)
at waffle.apache.MixedAuthenticator.<init>(Unknown Source)
at waffle.apache.MixedAuthenticatorTests.setUp(MixedAuthenticatorTests.java:45)
at org.slf4j.impl.JCLLoggerFactory.<clinit>(JCLLoggerFactory.java:55)

Matching master
Added mostly @OverRide and some other minor cleanup determine through
eclipse checks.
These showed up as flagged different likely due to my machine failure.
Updated to get off list.
This update resulted in some changes to test base code.
old tomcat setup.
Slf4j package only requires use of jcl over slf4j for spring, groboutils
and slf4j for remainder.
@hazendaz
Copy link
Member Author

hazendaz commented Jul 2, 2012

I'm still fuzzy on merging and getting sync'd to master. Let me know if anything else looks wrong at this point. I adjusted the change log to match the master now but did so manually.

OK - For Ant build...took a while to figure this out but turned out to be obvious issue. Slf4j package only needed two jars applied for the ant build otherwise it gets bind errors as it cannot bind to all types. While trying to figure this out, I turned on a lot of eclipse checks to help me out and as such noticed lots of @OVERRIDES missing so I added those and did some other very minor code cleanup. I went ahead and updated tomcat 6.0.29 to 6.0.35 along with associated code (ie additional overrides in test) and third party jars here. I also went ahead and added remaining jars missing from slf4j/logback since that appears how the other third parties are done.

@dblock
Copy link
Collaborator

dblock commented Jul 2, 2012

You still haven't properly merged - otherwise this page would not say that "This pull request cannot be automatically merged." Add upstream as a remote with git remote add [email protected]:dblock/waffle.git upstream, then make sure to do git pull upstream master, merge changes and push to your branch.

<classpathentry kind="lib" path="/ThirdParty/tomcat/lib/tomcat-coyote.jar"/>
<classpathentry kind="lib" path="/ThirdParty/tomcat/bin/tomcat-juli.jar"/>
<classpathentry kind="lib" path="/ThirdParty/slf4j/slf4j-1.6.6/slf4j-api-1.6.6.jar"/>
<classpathentry kind="lib" path="C:/Users/Jeremy Landis/My Programs/Java/slf4j/slf4j-1.6.6/jcl-over-slf4j-1.6.6.jar"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a ThirdParty path, not an absolute path to C:/Users/Jeremy Landis/....

@dblock
Copy link
Collaborator

dblock commented Jul 2, 2012

Note that it looks like there're a ton of changes here, that's because your spaces/tabs setting is different. You can see a file diff cleaner with https://github.com/dblock/waffle/pull/3/files?w=1, but it would be nice if you didn't change all the spaces to tabs or vice-versa.

@hazendaz
Copy link
Member Author

hazendaz commented Jul 2, 2012

Hmmm. I didn't see tab space differences when checking this link. I did update tomcat. Dies that account for the differences? I simply switched out 6.0.29 with 6.0.35 which accounted for what appeared to be the most changes here.

Sent from my Verizon Wireless Phone

--- Original Message ---

From: "Daniel Doubrovkine (dB.)" [email protected]
Sent: July 2, 2012 7/2/12
To: [email protected]
Subject: Re: [waffle] Slf4j (#3)

Note that it looks like there're a ton of changes here, that's because your spaces/tabs setting is different. You can see a file diff cleaner with https://github.com/dblock/waffle/pull/3/files?w=1, but it would be nice if you didn't change all the spaces to tabs or vice-versa.


Reply to this email directly or view it on GitHub:
#3 (comment)

@hazendaz
Copy link
Member Author

hazendaz commented Jul 2, 2012

Ill try to resolve tonight. I've been using the new 1.0 gui for github. This I assume is part of my issue. My Head also got corrupted last night after a cpu crash. So I had to rebuild. Thanks for all the feedback.

Sent from my Verizon Wireless Phone

--- Original Message ---

From: "Daniel Doubrovkine (dB.)" [email protected]
Sent: July 2, 2012 7/2/12
To: [email protected]
Subject: Re: [waffle] Slf4j (#3)

You still haven't properly merged - otherwise this page would not say that "This pull request cannot be automatically merged." Add upstream as a remote with git remote add [email protected]:dblock/waffle.git upstream, then make sure to do git pull upstream master, merge changes and push to your branch.


Reply to this email directly or view it on GitHub:
#3 (comment)

@dblock
Copy link
Collaborator

dblock commented Jul 2, 2012

Lmk if that turns out to be too annoying, I can just do it myself this side. The tabs/spaces have changed for all files, for example build.xml. Check the pull request on github to see what I see.

@hazendaz
Copy link
Member Author

hazendaz commented Jul 2, 2012

Ok ill fix any of those that are like that. I just started using eclipse 4.2. I didn't change defaults there but can understand it is defaulted differently. Thanks.

Sent from my Verizon Wireless Phone

--- Original Message ---

From: "Daniel Doubrovkine (dB.)" [email protected]
Sent: July 2, 2012 7/2/12
To: [email protected]
Subject: Re: [waffle] Slf4j (#3)

Lmk if that turns out to be too annoying, I can just do it myself this side. The tabs/spaces have changed for all files, for example build.xml. Check the pull request on github to see what I see.


Reply to this email directly or view it on GitHub:
#3 (comment)

Pointed to third party directory not locally.
@hazendaz
Copy link
Member Author

hazendaz commented Jul 3, 2012

I do see on some page diff's that appears to have some issue with formatting but on other pages, the same files look fine. When I try to edit the files, it looks like the formatting is identical. On a few of the ones you mentioned with issue such as the classpath, I didn't use eclipse to edit but rather notepad so unsure how it would have gotten new formatting. I'm building everything separately than the files git hub created on my machine. I did see where to do a merge and merged the change log. It did so against my master though which is where I really get confused as I know that is different from your master in at least the change log prior to me forcing the format. I'm still completely confused on how to get this stuff working properly. This doesn't seem that simple and easy as I had expected it to be. So I put in a few hours tonight trying to resolve things but do not believe I've gotten anywhere. I did fix the files you mentioned in issue. I see there are others still referencing commons-logging such as Waffle.proj. However, I'm concerned what I'm doing isn't sufficient so I'm holding off anything else until I can fully understand what I'm doing incorrectly. Based on the time I can put into this, this may be a hit or miss for the next few days.

I have git 1.7.11 & github 1.0 that I'm using. I'm sure I"m close to figuring everything out but frustrated at the moment. I definitely don't want to change formatting and feel I've inadvertently done so with no current understanding of how to resolve. Thanks for being so patient.

@dblock
Copy link
Collaborator

dblock commented Jul 3, 2012

I'm merging this, thanks for the hard work.

You might want to learn git command line, it's often clearer. Also in git it's easier to kill a branch and retry when you think you're in a dead end, branches are cheap :)

@dblock dblock merged commit 132dfa4 into Waffle:master Jul 3, 2012
@hazendaz
Copy link
Member Author

hazendaz commented Jul 3, 2012

Thanks. I sort of figured I was going to need to do this command line. Ill start 100% clean for work after this.

Sent from my Verizon Wireless Phone

--- Original Message ---

From: "Daniel Doubrovkine (dB.)" [email protected]
Sent: July 3, 2012 7/3/12
To: [email protected]
Subject: Re: [waffle] Slf4j (#3)

I'm merging this, thanks for the hard work.

You might want to learn git command line, it's often clearer. Also in git it's easier to kill a branch and retry when you think you're in a dead end, branches are cheap :)


Reply to this email directly or view it on GitHub:
#3 (comment)

@hazendaz
Copy link
Member Author

hazendaz commented Jul 4, 2012

Daniel,

Now that you have this merged, I took a look. Now I see everywhere that it appeared everything was removed and re-added. I think it might possibly have happened when I let eclipse auto-fix warnings such as the missing @OverRide. I will not do that in the future in case that is what caused it. To my much confusion, it definitely didn't appear that way when pulling the link you provided or when I committed. Next time if this appears to happen, I'll send you a screen shot of what I'm seeing. Hopefully by just avoiding the auto-fix feature in eclipse the issue might be resolved. Thanks for taking the merge as-is though. Hopefully others are not confused by what looks like a lot of changes.

Thanks,

Jeremy

-----Original Message-----
From: Daniel Doubrovkine (dB.) [mailto:[email protected]]
Sent: Tuesday, July 03, 2012 8:08 AM
To: hazendaz
Subject: Re: [waffle] Slf4j (#3)

I'm merging this, thanks for the hard work.

You might want to learn git command line, it's often clearer. Also in git it's easier to kill a branch and retry when you think you're in a dead end, branches are cheap :)


Reply to this email directly or view it on GitHub:
#3 (comment)


No virus found in this message.
Checked by AVG - www.avg.com
Version: 2012.0.2193 / Virus Database: 2437/5109 - Release Date: 07/03/12

@dblock
Copy link
Collaborator

dblock commented Jul 4, 2012

Don't worry about it, it's all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants