search

PowerCLIGoodies / DRSRule

PowerShell module with support for managing vSphere DRS Rules and Groups
28 stars 9 forks source link

Fix for non-default install directory #3

Open NamedJason opened 8 years ago

NamedJason commented 8 years ago

This will allow the module to work if PowerCLI is installed in a non-default directory (or if the workstation is 32 bit Windows).

lucdekens commented 8 years ago

Nice fix. Just one remark, the code assumes there is a module, so it won't work with PowerCLI 5.x. Would we need to have PowerCLI 5.x support? Thoughts? Luc

mtboren commented 8 years ago

Yes, good catch -- thanks for creating a fix, @NamedJason.

My thoughts on this not working for PowerCLI 5.x: PowerCLI 6.x has been out since Mar 2015, so it seems that there has been ample time for people to get current with their PowerCLI install. And, if there are still some people that, for some reason, "must" stay on a PowerCLI 5.x version, they can stay on v1.0.1 of the DRSRule module...

As for the bit in the updated code that "creates" the path to the given .dll from the module path, I suggest that we leverage some path-management cmdlets like Split-Path and Join-Path, staying out of the substring() game if we can. Like, instead of: $pcliDll = "$($modulePath.substring(0,$modulePath.indexOf('Modules')))VMware.Vim.dll"

use something like: $pcliDll = Join-Path -Path ((Get-Module VMware.VimAutomation.Core).ModuleBase | Split-Path -Parent | Split-Path -Parent) -ChildPath "VMware.Vim.dll"

A bit longer, but keeps us away from manual string manipulation.

Matt

NamedJason commented 8 years ago

How about this?

$pcliDLL = join-path -path (get-installpath) -childpath "VMware.Vim.dll"

I'm skeptical of it because it seems too simple...

lucdekens commented 8 years ago

Ok, with the PowerCLI 6.x requirement, but then we should add something like this.

Requires -Modules VMware.VimAutomation.Core, @{ModuleName="VMware.VimAutomation.Core";ModuleVersion="6.0.0.0"}

NamedJason commented 8 years ago

I'm not super familiar with GitHub, but I think that I changed this pull request to use that #Requires statement and the join-path (get-installpath) technique to populate pcliDll.