Skip to content

Implementation of -lp alias for -LiteralPath Parameter #6732#6770

Merged
TravisEz13 merged 1 commit intoPowerShell:masterfrom
kvprasoon:master
May 3, 2018
Merged

Implementation of -lp alias for -LiteralPath Parameter #6732#6770
TravisEz13 merged 1 commit intoPowerShell:masterfrom
kvprasoon:master

Conversation

@kvprasoon
Copy link
Copy Markdown
Contributor

@kvprasoon kvprasoon commented Apr 28, 2018

Implementation of -lp alias for -LiteralPath variable for issue #6732


PR Summary

Added test case for -lp alias usage for below cmdlets

Add-Content

Add-Type

Clear-Content

Clear-Item

Clear-ItemProperty

Convert-Path

Copy-Item

Export-Alias

Export-Clixml

Export-Csv

Export-FormatData

Format-Hex

Get-ChildItem

Get-Content

Get-FileHash

Get-Item

Get-ItemProperty

Get-ItemPropertyValue

Import-Alias

Import-Clixml

Import-Csv

Import-PowerShellDataFile

Move-Item

Out-File

Push-Location

Remove-Item

Rename-Item

Resolve-Path

Select-String

Select-Xml

Set-Content

Set-Item

Set-ItemProperty

Set-Location

Split-Path

Start-Job

Start-Transcript

Tee-Object

Test-Path

Unblock-File

PR Checklist

@kvprasoon
Copy link
Copy Markdown
Contributor Author

kvprasoon commented Apr 29, 2018

Fixed unit test failures for
Get-Content
Set-Content
Move-Item
Rename-Item
Tee-Object
Add-Type
cmdlets

iSazonov
iSazonov reviewed Apr 29, 2018
View reviewed changes
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

Leave a comment

PowerShell.Common.props Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Please revert changes in the file.
In your editor you could turn on a feature to auto remove trail spaces.

test/powershell/Modules/Microsoft.PowerShell.Core/Job.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please remove all alias tests - we have already special tests for aliases and no need to add them here.

iSazonov
iSazonov reviewed Apr 29, 2018
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Management/Get-Content.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Management/Move-Item.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Management/Rename-Item.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Management/Set-Content.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Utility/Add-Type.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Utility/Tee-Object.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

test/powershell/Modules/Microsoft.PowerShell.Utility/Tee-Object.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please fix.

iSazonov
iSazonov reviewed Apr 29, 2018
View reviewed changes
test/powershell/Modules/Microsoft.PowerShell.Utility/Add-Type.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Please remove trail spaces.

test/powershell/Modules/Microsoft.PowerShell.Utility/Tee-Object.Tests.ps1 Outdated
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 29, 2018

Choose a reason for hiding this comment

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

Seems this two test don't related to the PR.

Copy link
Copy Markdown
Contributor Author

@kvprasoon kvprasoon Apr 29, 2018

Choose a reason for hiding this comment

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

@iSazonov yes and those are existing .

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 30, 2018

Choose a reason for hiding this comment

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

You should keep the commit history and do rebase more accurately.

Copy link
Copy Markdown
Contributor Author

@kvprasoon kvprasoon Apr 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

👍 Sure and I'm learning 😊

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Apr 30, 2018

Choose a reason for hiding this comment

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

I suggest you to squash all your commits and then rebase your working branch.

Copy link
Copy Markdown
Contributor Author

@kvprasoon kvprasoon Apr 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, as of now all changes are committed.

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented Apr 30, 2018

I think I fixed the one test you have in my master branch here: https://github.com/travisez13/PowerShell ,but why did you remove the rest of the tests?

@kvprasoon
Copy link
Copy Markdown
Contributor Author

kvprasoon commented May 1, 2018 •
edited
Loading

@TravisEz13 All the tests ?, If you are referring tests for the new alias, then I've initially added tests for -lp alias for all the cmdlets, iSazonov asked me to remove all the tests I added for -lp alias as alias tests are taken care separately, hence removed.

please correct me if I'm wrong.

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

ok... I can clean up the PR better with that knowledge... give me a bit.

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

I squashed the changes down to one commit with the tests removed.

@kvprasoon
Copy link
Copy Markdown
Contributor Author

kvprasoon commented May 1, 2018

@TravisEz13 Thanks and how did you do that ? I don't know how to do it.

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

@kvprasoon Very carefully...
I created a commit (this took two tries) that reverted the change to test that wasn't completely reverted and just squashed the changes. I then changed the author back to you (you wouldn't have to do that.)

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

@kvprasoon Can you ping me when the tests finish?

@kvprasoon
Copy link
Copy Markdown
Contributor Author

kvprasoon commented May 1, 2018

@TravisEz13 of course

@kvprasoon
Copy link
Copy Markdown
Contributor Author

kvprasoon commented May 1, 2018

@TravisEz13 Completed with one failure(failed for a test case for Test-Connection) in appveyor CI

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

restarted appveyor to get clean results

@TravisEz13 TravisEz13 added Review - Committee The PR/Issue needs a review from the PowerShell Committee WG-Cmdlets general cmdlet issues WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module WG-Cmdlets-Management cmdlets in the Microsoft.PowerShell.Management module labels May 1, 2018
@TravisEz13 TravisEz13 self-assigned this May 1, 2018
@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 1, 2018

Marked for committee review. cc @SteveL-MSFT

@SteveL-MSFT
Copy link
Copy Markdown
Member

SteveL-MSFT commented May 2, 2018

@TravisEz13 committee review is just to accept the new alias?

@daxian-dbw
Copy link
Copy Markdown
Member

daxian-dbw commented May 2, 2018

@SteveL-MSFT It's a change that affects many cmdlets, so I think committee might want to take a quick look.

@daxian-dbw
Copy link
Copy Markdown
Member

daxian-dbw commented May 2, 2018

@PowerShell/powershell-committee has reviewed and agreed to add the alias -LP for the parameter -LiteralPath.

@daxian-dbw daxian-dbw 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 May 2, 2018
@TravisEz13 TravisEz13 merged commit ec678be into PowerShell:master May 3, 2018
@kvprasoon kvprasoon mentioned this pull request May 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov iSazonov iSazonov approved these changes

@adityapatwardhan adityapatwardhan Awaiting requested review from adityapatwardhan adityapatwardhan is a code owner

@anmenaga anmenaga Awaiting requested review from anmenaga

@dantraMSFT dantraMSFT Awaiting requested review from dantraMSFT

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw daxian-dbw is a code owner

@Francisco-Gamino Francisco-Gamino Awaiting requested review from Francisco-Gamino

@JamesWTruher JamesWTruher Awaiting requested review from JamesWTruher

@mirichmo mirichmo Awaiting requested review from mirichmo

@PaulHigin PaulHigin Awaiting requested review from PaulHigin

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13 TravisEz13 is a code owner

Assignees

@TravisEz13 TravisEz13

Labels

Committee-Reviewed PS-Committee has reviewed this and made a decision WG-Cmdlets general cmdlet issues WG-Cmdlets-Management cmdlets in the Microsoft.PowerShell.Management module WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kvprasoon @TravisEz13 @SteveL-MSFT @daxian-dbw @iSazonov