Skip to content

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

Merged
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
SteveL-MSFT:use-pathisunc
Jan 23, 2019
Merged

Update code to use single method to check if path is UNC#8680
Aditya Patwardhan (adityapatwardhan) merged 1 commit into
PowerShell:masterfrom
SteveL-MSFT:use-pathisunc

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented Jan 18, 2019

Copy link
Copy Markdown
Member

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

Steve Lee (@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

Ilya (@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

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

@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit f7c25e8 into PowerShell:master Jan 23, 2019
@adityapatwardhan Aditya Patwardhan (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
Steve Lee (SteveL-MSFT) deleted the use-pathisunc branch January 23, 2019 17:49
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
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