GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Make Move-Item work with its -Include, -Exclude, and -Filter parameters by jeffbi · Pull Request #3878 · PowerShell/PowerShell

Conversation

jeffbi commented May 30, 2017

Copy link

Copy Markdown

Fixed #2385

Invoke the correct overload of SessionState.Path.GetResolvedPSPathFromPSPath, passing the cmdlet context object.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Make Move-Item work with its -Include, -Exclude, and -Filter paramete…

83b2d6e

…rs (#2385)

Invoke the correct overload of SessionState.Path.GetResolvedPSPathFromPSPath, passing the cmdlet context object.

Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label

May 30, 2017

jeffbi commented May 30, 2017

Copy link

Copy Markdown

Author

Ilya (@iSazonov) Can you take a look at the Appveyor failure? I'm looking at the details and they seem to show two failure points, neither of which have anything to do with this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented May 30, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

jeffbi I see the same in other PRs. Nightly builds is affected too.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) reviewed

May 30, 2017

View reviewed changes

Ilya (iSazonov) left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Original Issue say that the cmdlet works well but writes unexpected error message - I don't see the check in tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Update tests per code review.

5cc46d9

jeffbi commented May 30, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Author

Ilya (@iSazonov) Well, what it actually said what that the cmdlet successfully performed the action that was intended, then wrote an error. That's not quite the same as it worked well.

I've updated the tests to check that the Move-Item was successful, and also to verify that files meant to be unaffected were unaffected.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented May 30, 2017

Copy link

Copy Markdown

Collaborator

LGTM.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) requested changes

Jun 5, 2017

View reviewed changes

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Management/Move-Item.Tests.ps1 Outdated

		$booContent = "boo content"
		}
		BeforeEach {
		New-Item -ItemType Directory -Path $filterPath

Aditya Patwardhan (adityapatwardhan) Jun 5, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should be

New-Item -ItemType Directory -Path $filterPath | Out-Null

to avoid on console output of created item

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeffbi Jun 5, 2017

Copy link

Copy Markdown

Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Management/Move-Item.Tests.ps1 Outdated

		It "Can move to different directory, filtered with -Include" {
		Move-Item -Path $filePath -Destination $moveToPath -Include "bar*"
		$? \| Should Be $true
		Test-Path -Path $barPath \| Should Be $false

Aditya Patwardhan (adityapatwardhan) Jun 5, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Use Powershell $barPath | Should Not Exist"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeffbi Jun 5, 2017

Copy link

Copy Markdown

Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Management/Move-Item.Tests.ps1 Outdated

		}
		It "Can move to different directory, filtered with -Include" {
		Move-Item -Path $filePath -Destination $moveToPath -Include "bar*"
		$? \| Should Be $true

Aditya Patwardhan (adityapatwardhan) Jun 5, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Difficult to debug from logs. Instead use

Move-Item -Path $filePath -Destination $moveToPath -Include "bar*" -ErrorVariable e -ErrorAction SilentlyContinue
$e | Should BeNullOrEmpty

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

jeffbi Jun 5, 2017

Copy link

Copy Markdown

Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Changes per code review.

992fe42

Aditya Patwardhan (adityapatwardhan) approved these changes

Jun 5, 2017

View reviewed changes

Mike Richmond (mirichmo) assigned Travis Plunk (TravisEz13)

Jun 6, 2017

Ilya (iSazonov) reviewed

Jun 6, 2017

View reviewed changes

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Management/Move-Item.Tests.ps1 Outdated

		It "Can move to different directory, filtered with -Include" {
		Move-Item -Path $filePath -Destination $moveToPath -Include "bar*" -ErrorVariable e -ErrorAction SilentlyContinue
		$e \| Should BeNullOrEmpty
		#Test-Path -Path $barPath \| Should Be $false

Ilya (iSazonov) Jun 6, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please remove the comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jeffbi Jun 6, 2017

Copy link

Copy Markdown

Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for catching that. Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) commented Jun 6, 2017

Copy link

Copy Markdown

Collaborator

jeffbi Thanks for the fix!

LGTM. (After removing one unneeded comment)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Remove stray comment

970efa0

jeffbi commented Jun 6, 2017

Copy link

Copy Markdown

Author

Ilya (@iSazonov), Aditya Patwardhan (@adityapatwardhan) Thanks for the review.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Travis Plunk (TravisEz13) commented Jun 8, 2017

Copy link

Copy Markdown

Member

jeffbi Can you update or remove the company in your profile? If it is accurate, please email me internally about additional steps you need to take.

Aditya Patwardhan (@adityapatwardhan) Please make your Microsoft Organization membership public.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Travis Plunk (TravisEz13) merged commit 5ee4ec1 into PowerShell:master

Jun 23, 2017

Mike Richmond (mirichmo) added a commit that referenced this pull request

Jun 23, 2017


          Revert "Make Move-Item work with its -Include, -Exclude, and -Filter …

76d4888

…parameters (#3878)"

This reverts commit 5ee4ec1.

jeffbi deleted the move-item-2385 branch

July 11, 2017 19:24

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Make Move-Item work with its -Include, -Exclude, and -Filter paramete…

6aaf735

…rs (PowerShell#3878)

* Make Move-Item work with its -Include, -Exclude, and -Filter parameters (PowerShell#2385)

Invoke the correct overload of SessionState.Path.GetResolvedPSPathFromPSPath, passing the cmdlet context object.

* Update tests per code review.

* Changes per code review.

* Remove stray comment

Reviewers

Ilya (iSazonov)

iSazonov left review comments

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Travis Plunk (TravisEz13)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

jeffbi commented May 30, 2017

Uh oh!

jeffbi commented May 30, 2017

Uh oh!

Ilya (iSazonov) commented May 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

jeffbi commented May 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) commented May 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) commented Jun 6, 2017

Uh oh!

jeffbi commented Jun 6, 2017

Uh oh!

Travis Plunk (TravisEz13) commented Jun 8, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Ilya (iSazonov) commented May 30, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

jeffbi commented May 30, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.