Skip to content

Comments

Send-MailMessage: add parameter "ReplyTo"#8727

Merged
anmenaga merged 2 commits intoPowerShell:masterfrom
replicaJunction:master
Jan 31, 2019
Merged

Send-MailMessage: add parameter "ReplyTo"#8727
anmenaga merged 2 commits intoPowerShell:masterfrom
replicaJunction:master

Conversation

@replicaJunction
Copy link
Contributor

@replicaJunction replicaJunction commented Jan 23, 2019

PR Summary

Adds a -ReplyTo parameter to Send-MailMessage, which allows users to include the Reply-To field in their e-mail messages.

PR Context

Fixes issue #8719.

PR Checklist

@msftclas
Copy link

msftclas commented Jan 23, 2019

CLA assistant check
All CLA requirements met.

@iSazonov iSazonov added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jan 24, 2019
@iSazonov
Copy link
Collaborator

@SteveL-MSFT Public API need approval by PowerShell Committee .

@iSazonov
Copy link
Collaborator

@replicaJunction Thanks for your contribution!

Please add tests (see Send-MailMessage.Tests.ps1 file).

@replicaJunction
Copy link
Contributor Author

@iSazonov I may need help adding tests for this. I did test interactively, and the reply-to field was correctly populated in an e-mail message, but even the existing Pester tests won't run on my development box since they're hard-coded to run only on Linux.

If I'm reading correctly, it looks like the existing tests actually call a mail server running on the local machine to send a message, and then they read the message back from disk. Could someone with easy access to a Linux device help with Pester tests?

@iSazonov
Copy link
Collaborator

@replicaJunction You could create new test like existing in Send-MailMessage.Tests.ps1 and push the commit - then you'll see result in CI (click Detail link).

@replicaJunction
Copy link
Contributor Author

I've added the -ReplyTo parameter to the existing tests, and it looks like they passed.

Regarding the CodeFactor issue above, I wrote the line in question to match the style of the existing code in that switch block. I don't think that issue can be fixed without reformatting the entire block, and I didn't want to clutter up this PR with formatting changes to others' code.

@anmenaga
Copy link

@SteveL-MSFT FYI. Waiting for PowerShell Committee to review this simple new public API.

@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this. It should be understood that cmdlets are not considered APIs, so adding a new parameter that is implemented as a public member does not require @PowerShell/powershell-committee review unless the reviewers of the PR believe it is controversial or can not come to a consensus. In this case, -ReplyTo makes sense.

@SteveL-MSFT SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jan 30, 2019
@anmenaga
Copy link

Exception for the CodeFactor issue seems reasonable. Merging,..

@anmenaga anmenaga merged commit c935bce into PowerShell:master Jan 31, 2019
@iSazonov
Copy link
Collaborator

@replicaJunction Thanks for your contribution!

@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants