Add OutFile property in WebResponseObject#24047
Add OutFile property in WebResponseObject#24047daxian-dbw merged 5 commits intoPowerShell:masterfrom
Conversation
b60059c to
9dd6213
Compare
iSazonov
left a comment
There was a problem hiding this comment.
Please add tests for the scenario in WebCmdlets.Tests.ps1.
...ft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebResponseObject.Common.cs
Outdated
Show resolved
Hide resolved
…ll now be saved to the WebResponseObject when -OutFile flag is used. Added Tests for directory, file, and verbose. Co-authored-by: Ilya <[email protected]>
fdf53e3 to
40781e0
Compare
|
@mklement0 Please review. |
| /// <summary> | ||
| /// Gets or sets the output file path. | ||
| /// </summary> | ||
| public string? OutFile { get; set; } |
There was a problem hiding this comment.
should this be an internal set?
There was a problem hiding this comment.
I agree (public string? OutFile { get; internal set; })
@SteveL-MSFT, @iSazonov, just to confirm: Is it generally fine to extend public classes with new members?
Other than that, LGTM - thanks for taking this on, @jshigetomi.
There was a problem hiding this comment.
@mklement0 Removing public element is a breaking change, adding is ok.
There was a problem hiding this comment.
@iSazonov Thank you for bringing this to my attention. I misread this chain and thought there were no actionable items.
|
@jshigetomi Please resolve merge conflicts. |
…Shell into issue-21082-fix
daxian-dbw
left a comment
There was a problem hiding this comment.
I guess the Invoke-ResetMethod -OutFile -Passthru needs to be updated accordingly to have the similar behavior, doesn't it?
...Shell.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs
Show resolved
Hide resolved
I didn't find But I found |
The |
|
@daxian-dbw: If it did / once it does work as intended, what would be passed through is data (possibly parsed into a |
...Shell.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs
Outdated
Show resolved
Hide resolved
…Cmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs
|
@mklement0 Thanks for pointing to that issue. I didn't know it's currently broken. I re-opened it. |
|
📣 Hey @jshigetomi, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Fixes: #21082
Added OutFile as a property in WebResponseObject. OutFile will save whenever writing to pipe line.
Refactored one line to reduce redundancy.
PR Context
PR extends WebObjectResponse with a property that stores the installed file's path. It makes sense to have this as a built in property since we have access to it.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.- [ ] Issue filed:
(which runs in a different PS Host).