search

serilog / serilog-sinks-email

A Serilog sink that writes events to SMTP email
Apache License 2.0
73 stars 68 forks source link

Added subject templating #26

Closed lloydpowell88 closed 7 years ago

lloydpowell88 commented 7 years ago

The subject line now works similarly to the message content, rather than being a static string, it can now contain templated areas such as {Level}, {MachineName} and any of the other placeholders.

adamchester commented 7 years ago

LGTM 👍

nblumhardt commented 7 years ago

@adamchester the EmailConnectionInfo type seems out of step with most other Serilog libraries now - what are your thoughts on it? Should we open an up-for-grabs issue for someone to deprecate it/move the args to the extension methods directly?

adamchester commented 7 years ago

@nblumhardt yes I think I see what you mean. Are you thinking about its impact on configuration (aside from it being a different/inconsistent pattern)?

nblumhardt commented 7 years ago

@adamchester yes; that and general "learnability". Having the API shaped consistently also makes things like the analyzer more useful.

adamchester commented 7 years ago

👍

adamchester commented 7 years ago

I also wonder now if MailKit should be a separate sink. As both sinks needed the same changes in this PR. Maybe we need an IEmailTransport

nblumhardt commented 7 years ago

@adamchester 👍 raised both.

lloydpowell88 commented 7 years ago

I agree with the MailKitseparation. The changes were identical, although if they had been more intricate then specific knowledge in that area would need to be gathered to make the change to the ordinary EmailSink.

Either way, I've made those minor changes and pushed them up. Thanks for that!