Skip to content

Send-MailMessage: add parameter "ReplyTo" - #8727

Merged
Andrew (anmenaga) merged 2 commits into
PowerShell:masterfrom
replicaJunction:master
Jan 31, 2019
Merged

Send-MailMessage: add parameter "ReplyTo"#8727
Andrew (anmenaga) merged 2 commits into
PowerShell:masterfrom
replicaJunction:master

Conversation

@replicaJunction

@replicaJunction Joshua T (replicaJunction) commented Jan 23, 2019

Copy link
Copy Markdown
Contributor

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

Microsoft Contribution License Agreements (msftclas) commented Jan 23, 2019

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@iSazonov Ilya (iSazonov) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jan 24, 2019
@iSazonov

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) Public API need approval by PowerShell Committee .

@iSazonov

Copy link
Copy Markdown
Collaborator

Joshua T (@replicaJunction) Thanks for your contribution!

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

@replicaJunction

Copy link
Copy Markdown
Contributor Author

Ilya (@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
Copy Markdown
Collaborator

Joshua T (@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
Copy Markdown
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
Copy Markdown

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

@SteveL-MSFT

Copy link
Copy Markdown
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 Steve Lee (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
Copy Markdown

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

@anmenaga
Andrew (anmenaga) merged commit c935bce into PowerShell:master Jan 31, 2019
@iSazonov

Copy link
Copy Markdown
Collaborator

Joshua T (@replicaJunction) Thanks for your contribution!

@iSazonov Ilya (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
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
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