Skip to content

remove -computer parameter from get-process - #4960

Merged
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
SteveL-MSFT:process-computer
Oct 18, 2017
Merged

remove -computer parameter from get-process#4960
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
SteveL-MSFT:process-computer

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented Sep 30, 2017

Copy link
Copy Markdown
Member

Due to bug https://github.com/dotnet/corefx/issues/24357 [Process]::GetProcesses(computer) returns local processes. Since this previously relied on dotnet remoting, I don't think we should support this and users should use PSRP instead.

@SteveL-MSFT Steve Lee (SteveL-MSFT) added Breaking-Change breaking change that may affect users Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Sep 30, 2017
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 1, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

Although -ComputerName works with Get-Service, we should consider removing it from Get-Service, Remove-Service, and Set-Service so that users consistently rely on PSRP and only having that port open

@joeyaiello

Joey Aiello (joeyaiello) commented Oct 4, 2017

Copy link
Copy Markdown
Contributor

We don't have quorum with the @PowerShell/powershell-committee today, but I'm formally acking that I'm okay with this one.

@iSazonov

Ilya (iSazonov) commented Oct 5, 2017

Copy link
Copy Markdown
Collaborator

I'm continuing to use WMI/RPC in a corporate environment.
This is familiar and convenient, as well as meet existing corporate security settings - changing it requires too much expense.
Before removing this I'd prefer to get support -PSSession in all the cmdlets.

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 5, 2017

Copy link
Copy Markdown
Member Author

Ilya (@iSazonov) corefx has this in their 2.1.0 milestone so it won't be fixed by the time we get to final. If PSRP is allowed in your environment, you can always use Invoke-Command to run it remotely. I agree that long term, having an automatic -PSSession parameter would be best.

@iSazonov

Ilya (iSazonov) commented Oct 5, 2017

Copy link
Copy Markdown
Collaborator

Steve Lee (@SteveL-MSFT) Does it make sense to do automatic -PSSession parameter in 6.1.0 milestone?

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 5, 2017

Copy link
Copy Markdown
Member Author

It's certainly something we could look at. Perhaps you could open an new issue to track it.

@iSazonov Ilya (iSazonov) mentioned this pull request Oct 8, 2017
Closed
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

@PowerShell/powershell-committee reviewed this and agreed to remove the -ComputerName parameter from this and the *-Service cmdlets. We may reuse that parameter in the future for remoting over PSRP so it was best to remove it now in 6.0.0.

@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 Oct 12, 2017
@iSazonov

Ilya (iSazonov) commented Oct 12, 2017

Copy link
Copy Markdown
Collaborator

Should we be consistency and remove the parameter from all cmslets forcing users to use PSRemoting?

PS C:\Windows\system32> gcm -ParameterName ComputerName

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Cmdlet          Add-Computer                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Clear-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Connect-PSSession                                  3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Enter-PSSession                                    3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Get-Counter                                        3.0.0.0    Microsoft.PowerShell.Diagnostics
Cmdlet          Get-EventLog                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-HotFix                                         3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-Process                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-PSSession                                      3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Get-Service                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Get-WinEvent                                       3.0.0.0    Microsoft.PowerShell.Diagnostics
Cmdlet          Get-WmiObject                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Invoke-Command                                     3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Invoke-WmiMethod                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Limit-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          New-EventLog                                       3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          New-PSSession                                      3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Receive-Job                                        3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Receive-PSSession                                  3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Register-WmiEvent                                  3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-Computer                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-EventLog                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Remove-PSSession                                   3.0.0.0    Microsoft.PowerShell.Core
Cmdlet          Remove-WmiObject                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Rename-Computer                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Restart-Computer                                   3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Send-MailMessage                                   3.1.0.0    Microsoft.PowerShell.Utility
Cmdlet          Set-Service                                        3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Set-WmiInstance                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Show-EventLog                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Stop-Computer                                      3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Test-Connection                                    3.1.0.0    Microsoft.PowerShell.Management
Cmdlet          Write-EventLog                                     3.1.0.0    Microsoft.PowerShell.Management

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

Ilya (@iSazonov) it looks like you enumerated the Windows PowerShell cmdlets, but yes, we should remove it from the other cmdlets except the ones specific to remoting.

@SteveL-MSFT Steve Lee (SteveL-MSFT) mentioned this pull request Oct 12, 2017
Closed
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

Ilya (@iSazonov) I created #5090 as I think only the *-Service ones need it removed, the remaining ones are ok.

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

Andrew (@anmenaga) since this was approved by Committee to proceed, can you review?

anmenaga
Andrew (anmenaga) approved these changes Oct 12, 2017
View reviewed changes

@anmenaga Andrew (anmenaga) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Change looks ok;
But looking that there is no corresponding test change, it feels strange that there is no existing test for "get-process -computer".
Maybe it is a good idea to run feature tests on this change to be sure that tests won't break...

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 12, 2017

Copy link
Copy Markdown
Member Author

Andrew (@anmenaga) pretty sure this isn't being used in our tests at all since I found this via code coverage analysis, but will run feature tests anyways

adityapatwardhan
Aditya Patwardhan (adityapatwardhan) approved these changes Oct 12, 2017
View reviewed changes

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) 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

@SteveL-MSFT
[feature]
151d078 
remove -computer support since corefx Process.GetProcesses(computer) returns local processes
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Oct 18, 2017

Copy link
Copy Markdown
Member Author

Aditya Patwardhan (@adityapatwardhan) can we merge?

@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit 847846d into PowerShell:master Oct 18, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) deleted the process-computer branch November 12, 2017 05:17
This was referenced Dec 5, 2017
Merged
Merged
@matt9ucci Matt Gucci (matt9ucci) mentioned this pull request Feb 28, 2018
Merged
8 tasks
@iSazonov Ilya (iSazonov) mentioned this pull request Mar 5, 2020
Closed
@rjmholt Rob Holt (rjmholt) mentioned this pull request Apr 15, 2020
Closed
xtqqczze added a commit to xtqqczze/PowerShell-PowerShell that referenced this pull request Mar 17, 2021
@xtqqczze
Remove unnecessary Array -> List -> Array conversion
8316bed 
Change removes redundant code from ProcessBaseCommand.AllProcesse that was left after existing functionality was removed in PowerShell#4960.
@xtqqczze xtqqczze mentioned this pull request Mar 17, 2021
Merged
8 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@SteveL-MSFT @adityapatwardhan
[feature] (PowerShell#4960)
14be724 
remove -computer support since corefx Process.GetProcesses(computer) returns local processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw
+1 more reviewer
@anmenaga Andrew (anmenaga) anmenaga approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@SteveL-MSFT @joeyaiello @iSazonov @anmenaga @adityapatwardhan