Skip to content

Improve CommandInvocationIntrinsics API documentation and style#14369

Merged
daxian-dbw merged 3 commits intoPowerShell:masterfrom
rjmholt:commandinvocationintrinsics-documentation
Sep 3, 2021
Merged

Improve CommandInvocationIntrinsics API documentation and style#14369
daxian-dbw merged 3 commits intoPowerShell:masterfrom
rjmholt:commandinvocationintrinsics-documentation

Conversation

@rjmholt
Copy link
Copy Markdown
Collaborator

@rjmholt rjmholt commented Dec 10, 2020

PR Summary

Using this API, I found I had to inspect the source code to determine the behaviour of the overloads I was looking over.

I've added comments so that in future that won't be necessary.

I think it's particularly interesting that the string overloads execute in a local scope, while the ScriptBlock overloads dot-source — so this seems like an important thing to capture in documentation.

I've also hung the parameters and renamed one parameter because it was inconsistently named across overloads (reverted this second part since it's a breaking change for named parameters).

PR Context

PR Checklist

@ghost ghost assigned TravisEz13 Dec 10, 2020
Comment thread src/System.Management.Automation/engine/MshCmdlet.cs Outdated
Comment thread src/System.Management.Automation/engine/MshCmdlet.cs Outdated
Comment thread src/System.Management.Automation/engine/MshCmdlet.cs Outdated
@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Dec 12, 2020
@ghost ghost added the Review - Needed The PR is being reviewed label Dec 19, 2020
@ghost
Copy link
Copy Markdown

ghost commented Dec 19, 2020

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@daxian-dbw
Copy link
Copy Markdown
Member

@rjmholt @TravisEz13 Any changes needed for this PR?

@ghost ghost removed the Review - Needed The PR is being reviewed label Apr 26, 2021
Copy link
Copy Markdown
Member

@daxian-dbw daxian-dbw left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost added the Review - Needed The PR is being reviewed label May 7, 2021
@ghost
Copy link
Copy Markdown

ghost commented May 7, 2021

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@daxian-dbw daxian-dbw merged commit 17986d8 into PowerShell:master Sep 3, 2021
@ghost ghost removed the Review - Needed The PR is being reviewed label Sep 3, 2021
@iSazonov iSazonov added this to the 7.2.0-rc.1 milestone Sep 4, 2021
@ghost
Copy link
Copy Markdown

ghost commented Dec 16, 2021

🎉v7.3.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this pull request Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backport-7.2.x-Done CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants