Skip to content

Update code to use single method to check if path is UNC#8680

Merged
adityapatwardhan merged 1 commit intoPowerShell:masterfrom
SteveL-MSFT:use-pathisunc
Jan 23, 2019
Merged

Update code to use single method to check if path is UNC#8680
adityapatwardhan merged 1 commit intoPowerShell:masterfrom
SteveL-MSFT:use-pathisunc

Conversation

@SteveL-MSFT
Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT commented Jan 18, 2019

PR Summary

The code had a simple (potentially incorrect) check if path is UNC and other code relied on a pinvoke to a win32 api. There is an existing utility method to check if a path is UNC using the URI class. Update all code to use this single utility method.

PR Checklist

update all checks if path is unc to one method
@iSazonov
Copy link
Copy Markdown
Collaborator

The code had a simple (potentially incorrect)

It seems we have a gap in the tests :-(

@iSazonov
Copy link
Copy Markdown
Collaborator

@SteveL-MSFT I still believe that we need tests because we did not catch the problem previously in tests and we can create a regression.

@SteveL-MSFT
Copy link
Copy Markdown
Member Author

@iSazonov agree we have a test gap, but it's unclear where the simple verification of seeing if the path started with two backslashes would fail which is why I said "potentially". The FileSystem.Tests.ps1 has some UNC specific tests. This was more of a code cleanup than fixing a known issue so not sure what test to add here.

@iSazonov
Copy link
Copy Markdown
Collaborator

@SteveL-MSFT We'll find this when we create FileSystem provider V2. :-)

@adityapatwardhan adityapatwardhan merged commit f7c25e8 into PowerShell:master Jan 23, 2019
@adityapatwardhan adityapatwardhan added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Jan 23, 2019
@SteveL-MSFT SteveL-MSFT deleted the use-pathisunc branch January 23, 2019 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants