Defer logger creation to allow custom logging configuration #129
Conversation
There was a problem hiding this comment.
@SimonCropp @seanfarmar No real blockers. Should be backport this? As basically https://github.com/Particular/NServiceBus.Host/tree/hotfix-6.0.1 introduced this bug. It is not critical so based on our policy we shouldn't but personally it feels we should.
| @@ -32,9 +31,10 @@ public void Start() | |||
| { | |||
| genericHost.Start().GetAwaiter().GetResult(); | |||
| } | |||
| catch (Exception ex) | |||
| catch (Exception exception) | |||
There was a problem hiding this comment.
Is that our naming convention or is this personal style?
There was a problem hiding this comment.
descriptive variable names FTW
There was a problem hiding this comment.
Not against that in general but I tend to not apply refactorings in patches to keep diffs as clean as possible.
| { | ||
| Log.Fatal("Start failure", ex); | ||
| var log = LogManager.GetLogger<WindowsHost>(); |
There was a problem hiding this comment.
Shall we add a comment to prevent someone refactoring this back to a field?
|
i dont think it needs to be back ported |
ramonsmits
changed the title
|
Ok to merge this? |
|
if you have released 7.0.2 |
|
@Particular/host-maintainers any update? |
|
@SimonCropp Still needs to be released. I'll do my best to release this Tomorrow. |
this change c591c75
added the line
ILog Log = LogManager.GetLogger<WindowsHost>();This line will execute prior to the logging being configured by the user and result in a incorrect log log file created as described here https://groups.google.com/forum/#!topic/particularsoftware/7QK15QEMV4E
This PR defers the log creation until after the user has a chance to configure it