-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Simplification of GetHttpMethod and httpMethod in WebCmdlets #18846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplification of GetHttpMethod and httpMethod in WebCmdlets #18846
Conversation
...erShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs
Outdated
Show resolved
Hide resolved
d4441b1
to
2b6d084
Compare
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
4f45a1a
to
0828454
Compare
@CarloToso Of course cmdlets with lots of parameters and parameter sets look complicated nevertheless I don't see the benefit of removing these parameter sets. I am somewhat surprised that parameters are declared as virtual and we override them. We can't make them not virtual without PowerShell Committee approval but I guess we could remove the overrides. |
@iSazonov I added back the parameters set, is the rest of the code ok? |
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
I don't see value in the change. (Also it can break scripts working with meta data.) |
At the moment WebRequestMethod does not contain the CONNECT Http request method, should we do something about it? |
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
Please open new issue with investigation why we should add. |
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
@CarloToso Please update the PR description. |
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
🎉 Handy links: |
PR Summary
GetHttpMethod --> switch expression
Simplification of setting httpMethod
Remove IsStandardMethodSet(), IsCustomMethodSet()
PR Context
Improve readibility of the WebCmdlets, cleanup