Skip to content
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

Fixed resolving of path to support PSDrives #61

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

lipkau
Copy link
Contributor

@lipkau lipkau commented May 14, 2018

Resolve-Path does not resolve PSDrives, which are not supported as WorkingDirectory.
This was already observed in Pester:
pester/Pester#1029

lipkau added 4 commits May 14, 2018 09:04
`Resolve-Path` does not resolve PSDrives, which are not supported as WorkingDirectory.
This was already observed in Pester:
pester/Pester#1029
# Conflicts:
#	BuildHelpers/Public/Get-ProjectName.ps1
@@ -41,19 +43,19 @@ function Get-ModuleAliases {
$params = @{
Force = $True
Passthru = $True
Name = $Name
Name = Get-FullPath $Name

Choose a reason for hiding this comment

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

I assume this will always be a path, but comment based help could use a tweak to avoid confusion (i.e. it's not either a name or path, it's a path and only a path)?

@@ -41,19 +43,19 @@ function Get-ModuleFunctions {
$params = @{
Force = $True
Passthru = $True
Name = $Name
Name = Get-FullPath $Name

Choose a reason for hiding this comment

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

Same here - I assume this will always be a path, but comment based help could use a tweak to avoid confusion (i.e. it's not either a name or path, it's a path and only a path)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow.
CBH on Get-FullPath?
Or using a named parameter instead of positioned?

Copy link
Owner

@RamblingCookieMonster RamblingCookieMonster Jun 8, 2018

Choose a reason for hiding this comment

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

Oh sorry! To clarify, for all of these, they're used with Import-Module -Name - i.e. could be either a Path, or a Name. It really should always be a path in the context of buildhelpers, but comment based help for Name still mentions it: Name or path to module to inspect.

{
$ExpectedManifest
}
$ThisFolder = $_

Choose a reason for hiding this comment

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

Is there an auto formatter at work? Seeing some odd behavior (e.g. here I wouldn't expect foreach cmdlet to be in line with it's scriptblock content)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VS Code formatting.
I will fix this

@@ -44,17 +46,17 @@ function Set-ModuleAliases {
$params = @{
Force = $True
Passthru = $True
Name = $Name
Name = Get-FullPath $Name

Choose a reason for hiding this comment

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

Same note on comment based help

@RamblingCookieMonster
Copy link
Owner

Alrighty! Apart some whitespace / formatting oddities and a few comment based help things that aren't accurate (but are very unlikely to be used - can still bump version just in case), looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants