-
Notifications
You must be signed in to change notification settings - Fork 829
allow pushing a IEnumerable of ILogEventEnricher #2163
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
allow pushing a IEnumerable of ILogEventEnricher #2163
Conversation
Makes sense 👍 I think since there's little chance we'd actually remove the original array version in the foreseeable future, and considering there might be scenarios in which it generates a warning (e.g. old compiler versions?), perhaps we drop the obsolete attribute? Gave a bit of thought to whether we'd also want to provide a |
done
added i also added some tests |
i also removed the OverloadResolutionPriority. since if we want to keep the array varient around, there is no need to redirect |
nudge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 👍
Looks like CI is failing - stack overflow?
turns out i still need the OverloadResolutionPriority to point the compiler to the most efficient overload |
given in some contexts i build up a list of ILogEventEnricher of a (runtime) variable size. So the array nature of push forces a redundant array alloc.
this PR adds a
params IEnumerable
overload.I am not sure on having the Obsolete? the OverloadResolutionPriority will point the compiler to IEnumerable overload. so it is not a compile time breaking change. and it is not a runtime breaking change. thoughts?