Skip to content
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

Fix for issue #507: NLog optional or empty mail recipient #523

Merged
2 commits merged into from
Jan 27, 2015

Conversation

YuLad
Copy link
Contributor

@YuLad YuLad commented Jan 24, 2015

Changes summary:

  • Ignore empty recipient values by specifying the StringSplitOptions.RemoveEmptyEntries option.
  • Write to internal logger if email sending failed.
  • Add RequiredParameterAttribute to the SmtpServer property - it should be required according to the documentation. Breaking change?
  • Throw NLogRuntimeException if one of the required parameters (From, To, SmtpServer) becomes empty after rendering. Add relevant unit tests to verify the expected behaviour.
  • Also added some negative unit tests for MailTarget in the first commit.

Fixes #507


This change is Reviewable

Ignore empty recipient values by specifing the StringSplitOptions.RemoveEmptyEntries option.

Write to internal logger if email sending failed.

Add RequiredParameterAttribute to the SmtpServer property - it should be required according to the documentation -
https://github.com/NLog/NLog/wiki/Mail-target#user-content-smtp-options.

Throw NLogRuntimeException if one of the required parameters (From. To, SmtpServer) becomes empty after rendering. Add relevant unit tests to verify the expected behaviour.
@YuLad YuLad changed the title Fix for issue 507: NLog optional or empty mail recipient Fix for issue #507: NLog optional or empty mail recipient Jan 24, 2015
@ghost
Copy link

ghost commented Jan 25, 2015

Looks very good and thorough. I'll merge soon

@ghost
Copy link

ghost commented Jan 25, 2015

Meaning as soon as I figured why appveyor doesn't report failing tests to Github

@ghost ghost added the bug Bug report / Bug fix label Jan 27, 2015
@ghost ghost added this to the 4.0 milestone Jan 27, 2015
@ghost ghost self-assigned this Jan 27, 2015
ghost pushed a commit that referenced this pull request Jan 27, 2015
Fix for issue #507: NLog optional or empty mail recipient
@ghost ghost merged commit 7bebcba into NLog:master Jan 27, 2015
@YuLad YuLad deleted the 507-emptyMailRecipient branch January 27, 2015 17:48
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix mail-target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants