Support AuditTo in configuration #87
Conversation
|
I don't know if it's worth incrementing the minor version -> v2.5.0 ... because of new feature, and also to align it with the version of Serilog that is referenced |
| @@ -260,6 +271,13 @@ internal static IList<MethodInfo> FindSinkConfigurationMethods(IEnumerable<Assem | |||
| return found; | |||
| } | |||
|
|
|||
| internal static IList<MethodInfo> FindAuditSinkConfigurationMethods(IEnumerable<Assembly> configurationAssemblies) | |||
| { | |||
| var found = FindConfigurationMethods(configurationAssemblies, typeof(LoggerAuditSinkConfiguration)); | |||
There was a problem hiding this comment.
All the other methods somehow used found.Add(GetSurrogateConfigurationMethod<T> , but I was not sure whether that was needed for Audit ..
There was a problem hiding this comment.
@tsimbalar GetSurrogateConfigurationMethod<T> is for methods mimicking static methods (extensions) and used just to get MethodInfo using expressions without reflection, but implementation I did is overcomplicated and just realized you can get MehtodInfo right out of method group after casting to appropriate delegate type:
((Func<LoggerFilterConfiguration, ILogEventFilter, LoggerConfiguration>)With).MethodInfo
There was a problem hiding this comment.
oh I see. I guess it might not be necessary for AuditTo
There was a problem hiding this comment.
I think it'd be needed for AuditTo.Logger() .. but this seems like a bit of a fringe case.
|
Thanks for the huge push on all of these things! I notice you're back from vacation :-) I think updating the minor version is reasonable - though probably not a huge deal for downstream consumers. Perhaps, since we're bumping the Serilog dependency version, we should update it to 2.6 so that the more recent package is brought in by default when installing this one? Thoughts? |
|
Ok I'll increase the version number and reference v2.6. |
and use https URLS in nuget package metadata
|
Looks great! 👍 |
|
btw, you may have noticed, but most of the test code is copied/pasted from the core Serilog repo :) |
|
@nblumhardt @skomis-mm just a heads up, I think I may have messed up at some point, and added the xml element I have added a commit to fix it afterwards (7d1f277) , but I think because of that change, a "non prerelease" nuget package has been published : https://www.nuget.org/packages/Serilog.Settings.Configuration/2.5.0 ... So we now have a "non-prerelease" nuget package that has been produced from the |
|
@tsimbalar I believe the only change should be is minor version bump (2.6) after LoggingLevelSwitch pr is merged since its adds new features. The published changes are minimal, tested, so there should be no problems. p.s. its nothing comparing how CsvHelper was evolved in last few months :) |
|
@tsimbalar I think the package out there as 2.5.0 is probably fine to leave, since it's been downloaded and used. Let's do as @skomis-mm suggests and bump the prefix to 2.6.0, which we can send through to |
Fixes #80