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
MessageTemplateParameter(s) ctors to internal #2581
Conversation
@snakefoot decline this and wontfix for #2552? |
I was thinking to allow use of MessageTemplateParameters constructor to access the internal parser. The MessageTemplateParameter struct is used by NLog.Extensions to capture Microsoft Extension Logging Message Properties. But we don't need two constructors, only the one with CaptureType
…Sent from my Samsung device
-------- Original message --------
From: Julian Verdurmen <notifications@github.com>
Date: 19/02/2018 21:19 (GMT+00:00)
To: NLog/NLog <NLog@noreply.github.com>
Cc: Rolf Kristensen <sweaty1@hotmail.com>, Mention <mention@noreply.github.com>
Subject: Re: [NLog/NLog] MessageTemplateParameter(s) ctors to internal (#2581)
@snakefoot<https://github.com/snakefoot> decline this and wontfix for #2552<#2552>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#2581 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AK-fnPuD2DYkB2IjFhfti5Q8UBfxywxPks5tWeVGgaJpZM4SLDZ8>.
|
So for both classes we need public ctors? |
14c5231
to
dbffc3f
Compare
Codecov Report
@@ Coverage Diff @@
## master #2581 +/- ##
=======================================
- Coverage 81% 81% -<1%
=======================================
Files 325 325
Lines 23918 23923 +5
Branches 3022 3023 +1
=======================================
- Hits 19403 19393 -10
+ Misses 3702 3698 -4
- Partials 813 832 +19 |
No really a fan of this magic. updated it: MessageTemplateParameter full ctor is still public MessageTemplateParameters is internal ctor. IMO the ctor of MessageTemplateParameters do maybe to much parsing and so it's a good candidate to keep it internal. Let me know if you agree We can always open the ctor if we need it in the future. |
PS I will try to create rc6 tonight/tomorrow. I hope that the latest RC before the RTM @snakefoot if your still on holiday, have a good holiday :) |
So for both classes we need public ctors?
Yes. I think so
|
MessageTemplateParameters is internal ctor. IMO the ctor of MessageTemplateParameters do maybe to much parsing and so it's a good candidate to keep it internal.
Will also work for me
|
fixes #2552