Skip to content

Conversation

@jamescarr
Copy link
Contributor

Commits speak for themselves. Low hanging fruit that I thought I'd take since I got bit by this very issue today.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should not log under warn after testing for isInfoEnabled (or isDebugEnabled). Always log at the same level as if test.

simply replace both with a single

if (logger.isWarnEnabled() {
logger.warn(...., ex);
}

(See JMS AMLC)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Just updated the commit range.

@garyrussell
Copy link
Contributor

James,

Before we go any further, have you reviewed and signed the Individual Contributor Agreement?

https://support.springsource.com/spring_committer_signup

Thanks

@jamescarr
Copy link
Contributor Author

Yes. I did when I contributed some changes last year.
On Nov 15, 2011 11:20 AM, "Gary Russell" <
[email protected]>
wrote:

James,

Before we go any further, have you reviewed and signed the Individual
Contributor Agreement?

https://support.springsource.com/spring_committer_signup

Thanks


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

Copy link
Contributor

Choose a reason for hiding this comment

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

Great!

You should remove this last else if{...} stanza; it will never be executed because the one above it will run.

Thanks.

@james-carr
Copy link

Changes made

@garyrussell
Copy link
Contributor

Mark,

LGTM - ok to merge (w/ squash).

garyrussell added a commit to garyrussell/spring-amqp that referenced this pull request Mar 23, 2012
garyrussell pushed a commit that referenced this pull request Apr 26, 2016
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.

3 participants