Skip to content

Fix regression caused by logging refactoring. #1479

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

Merged
merged 2 commits into from
Dec 13, 2017

Conversation

ngbrown
Copy link
Contributor

@ngbrown ngbrown commented Dec 8, 2017

Added back the unit tests for the legacy logging provider obsoleted by #1377.

Fixes #1478.

@hazzik
Copy link
Member

hazzik commented Dec 8, 2017

@lnu could you please check that it fixes your problem?

@hazzik hazzik added this to the 5.1 milestone Dec 8, 2017
@hazzik hazzik added the t: Fix label Dec 8, 2017
@lnu
Copy link
Member

lnu commented Dec 9, 2017

@hazzik @ngbrown I ran nhibernate.envers tests without the pull request and all the tests failed. With the pull all the tests are green(and the logs are working). It seems to fix the issue. I'll be able to test with a real app on tuesday but if all tests are green it's ok I guess.

NHibernateLogger.SetLoggersFactory(default(INHibernateLoggerFactory));

// NoLoggingNHibernateLogger is internal
Assert.That(LoggerProvider.LoggerFor("pizza"), Is.InstanceOf<NoLoggingInternalLogger>());
Copy link
Member

Choose a reason for hiding this comment

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

Irrelevant comment. And next one too.

@@ -52,6 +52,10 @@ public static class NHibernateLogger
private const string nhibernateLoggerConfKey = "nhibernate-logger";
private static INHibernateLoggerFactory _loggerFactory;

#pragma warning disable 618
internal static ILoggerFactory LegacyLoggerFactory { private set; get; }
Copy link
Member

Choose a reason for hiding this comment

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

A bit strange to see the setter before the getter. I would rather have it in the usual order.

@hazzik hazzik merged commit 906c692 into nhibernate:master Dec 13, 2017
@ngbrown ngbrown deleted the bug/1478 branch December 13, 2017 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants