Skip to content

Improve CommandInvocationIntrinsics API documentation and style#14369

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

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

Conversation

@rjmholt

@rjmholt rjmholt commented Dec 10, 2020

Copy link
Copy Markdown
Collaborator

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

ghost commented Dec 19, 2020

Copy link
Copy Markdown

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

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

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

ghost commented May 7, 2021

Copy link
Copy Markdown

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

ghost commented Dec 16, 2021

Copy link
Copy Markdown

🎉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