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.

Sign up for GitHub

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

Jump to bottom

Added the ability to allow virtual paths for SMTP pickup directory #1273

Merged
merged 7 commits into from
Mar 10, 2016
Merged

Added the ability to allow virtual paths for SMTP pickup directory #1273

merged 7 commits into from
Mar 10, 2016

Conversation

MikeBairdRocks
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Current coverage is 74.82%

Merging #1273 into master will increase coverage by +0.02% as of ed2af68

@@            master   #1273   diff @@
======================================
  Files          267     267       
  Stmts        15449   15458     +9
  Branches      1621    1622     +1
  Methods          0       0       
======================================
+ Hit          11557   11566     +9
  Partial        383     383       
  Missed        3509    3509

Review entire Coverage Diff as of ed2af68

Uncovered Suggestions

  1. +0.10% via ...gingConfiguration.cs#282...295
  2. +0.10% via ...gingConfiguration.cs#248...261
  3. +0.09% via ...argets/FileTarget.cs#1713...1725
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@304NotModified
Copy link
Member

Cool! Thanks!

@MikeBairdRocks
Copy link
Contributor Author

I did want to make a helper method for that code but did not want to introduce anything new w/o approval. 👍 I'll get an update to the PR sometime tomorrow.

@304NotModified 304NotModified self-assigned this Mar 3, 2016
@304NotModified 304NotModified added this to the 4.3 milestone Mar 3, 2016
304NotModified
304NotModified reviewed Mar 5, 2016
View reviewed changes
src/NLog/Targets/MailTarget.cs
/// <param name="client">client to set properties on</param>
/// <param name="pickupDirectoryLocation"></param>
/// <returns></returns>
internal static string GetPickupDirectoryLocation(ISmtpClient client, string pickupDirectoryLocation)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal:

  • remove 'ISmtpClient client' (I guess this is a bug)
  • rename to "ConvertDirectoryLocation"
  • update docs

@304NotModified
Copy link
Member

Thanks for the changes!

There are some improvements needed before I can merge it. See the code comments.

PS: I don't get an alert when commits are pushed to the PR.

@304NotModified 304NotModified removed their assignment Mar 7, 2016
@MikeBairdRocks
Copy link
Contributor Author

PR Updated

@304NotModified 304NotModified self-assigned this Mar 10, 2016
@304NotModified
Copy link
Member

Thanks!

304NotModified added a commit that referenced this pull request Mar 10, 2016
@304NotModified
Merge pull request #1273 from michaeljbaird/master
f7b4d4d 
Added the ability to allow virtual paths for SMTP pickup directory
@304NotModified 304NotModified merged commit f7b4d4d into NLog:master Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@304NotModified 304NotModified

Labels
feature mail-target
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@MikeBairdRocks @codecov-io @304NotModified @snakefoot