★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/pull/7910Nouvelle recherche | Portail wanayoo
Skip to content

/PowerShell

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

Remove clone args #7910

Merged
merged 3 commits into from Oct 29, 2018

Conversation

Reviewers

@TravisEz13 TravisEz13

@anmenaga anmenaga

@daxian-dbw daxian-dbw

@PaulHigin PaulHigin

Assignees
No one assigned
Labels
Issue-Code Cleanup
Projects
None yet
Milestone
No milestone
3 participants
@iSazonov @TravisEz13 @anmenaga
@iSazonov
Copy link
Collaborator

commented Oct 1, 2018 •
edited

PR Summary

Close #7457.

The cleanup is needed after #7449 and #2538.

  • args is never null
  • We never change the args array and exclude cloning.

PR Checklist

@iSazonov iSazonov added the Issue-Code Cleanup label Oct 1, 2018

@iSazonov iSazonov requested review from TravisEz13 and PaulHigin Oct 1, 2018

@iSazonov iSazonov requested review from anmenaga and daxian-dbw as code owners Oct 1, 2018

@TravisEz13

This comment has been minimized.

Sign in to view
Copy link
Member

commented Oct 1, 2018

@daxian-dbw Not sure why this was there in the first place.
@anmenaga

anmenaga requested changes Oct 10, 2018
View changes

Copy link
Contributor

left a comment •
edited

It would be good to get a [feature] test pass on this. Thanks.

Also, @daxian-dbw any ideas regarding Travis's question? I wonder the same thing as well...
@TravisEz13

TravisEz13 reviewed Oct 10, 2018
View changes

src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs
@@ -163,16 +160,11 @@ internal sealed partial class ConsoleHost
hostException = e;
}

if (args == null)

This comment has been minimized.

Sign in to view
Copy link
@TravisEz13

TravisEz13 Oct 10, 2018

Member

We should probably leave the null check in the code.

This comment has been minimized.

Sign in to view
Copy link
@iSazonov

iSazonov Oct 11, 2018

Author Collaborator

Is c# convention that args is never null?

iSazonov added some commits Oct 1, 2018

@iSazonov
Remove dumb code 
- args is never null
- the code is not used
Loading status checks…
28b3ff1
@iSazonov
Remove unneeded args cloning
Loading status checks…
ae7154f
@iSazonov
[Feature]
Loading status checks…
d0d0300

@iSazonov iSazonov force-pushed the iSazonov:remove-clone-args branch to d0d0300 Oct 12, 2018

@anmenaga

anmenaga approved these changes Oct 25, 2018
View changes

@iSazonov

This comment has been minimized.

Sign in to view
Copy link
Collaborator Author

commented Oct 26, 2018

@daxian-dbw Can you confirm that we can merge?

@TravisEz13 TravisEz13 merged commit c36e4ad into PowerShell:master Oct 29, 2018
8 checks passed

8 checks passed

CodeFactor No issues found.
Details
PowerShell-CI-linux #PR-7910-20181012.01 succeeded
Details
PowerShell-CI-macos #PR-7910-20181012.01 succeeded
Details
PowerShell-CI-spelling #PR-7910-20181012.01 succeeded
Details
PowerShell-CI-windows #PR-7910-20181012.01 succeeded
Details
WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla All CLA requirements met.

@iSazonov iSazonov deleted the iSazonov:remove-clone-args branch Oct 30, 2018

adityapatwardhan pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Apr 9, 2019

@iSazonov @TravisEz13
Cleanup: Remove clone of command-line args array (PowerShell#7910)
ad2f7a5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.