joel74 / POSH-LTM-Rest

PowerShell module and scripts for working with F5's LTM REST API
MIT License
77 stars 48 forks source link

Function Naming Collisions #145

Open hackjammer opened 6 years ago

hackjammer commented 6 years ago

Many of the module functions have pretty generic names, that do clash with other modules and may clash with more in future, e.g. New-Node (https://www.powershellgallery.com/items?q=new-node) Get-Pool (https://www.powershellgallery.com/items?q=get-pool)

I would suggest using a prefix across all of the function, such as "F5" e.g. New-F5Node This can be done simply by specifying "DefaultCommandPrefix" in the module psd1

For reference see e.g. https://www.sapien.com/blog/2016/02/15/use-prefixes-to-prevent-command-name-collision/

joel74 commented 6 years ago

True, it would be a simple change, but it would also be a huge breaking change. Maybe if there were a way to preserve the name via an alias and make the base function name include F5 (where it doesn't already), but that probably wouldn't resolve any possible name collision issues. If I were starting the module again from scratch, I'd most likely aim to make the function names a bit more F5-specific, but I don't feel that a change of that magnitude and disruption is the best idea right now.

vercellone commented 6 years ago

Team\Get-Pool bit me today. @DarqueWarrior Set-Alias Get-Pool Get-VSTeamPool. But, the conflict exists because neither of us are following the rules. I agree that all our functions should be prefixed F5 (not BIGIP). I also prefer not to include LTM/GTM except when (if) absolutely necessary to distinguish LTM and GTM variants of the same object/command. And, in that case it may be better to use F5DNS for the GTM variants and just use F5 for the LTM variants.

Why not use #138 as a jumping off point to start a new project/repository with proper naming conventions? When that matures, this module could be deprecated.

joel74 commented 6 years ago

@vercellone - we do already have BIGIP as a possible jumping-off point. I had hoped this new module would require minimal breaking changes, but in theory we could keep all the LTM functions identical between this LTM module and that BIGIP module, and just add an 'F5' prefix to the BIGIP functions.

mcurole commented 6 years ago

How about making the breaking change to conform to the naming standards but provide a script that could be dot sourced to add aliases to the old names? This could optionally be used to keep compatibility with old scrips.

joel74 commented 6 years ago

I like that idea. Maybe provide instructions to dot-source it in the module definition, but don't set that up by default.

riveryc commented 6 years ago

What about using New-Alias Get-Pool Get-F5Pool To make sure the compatibility. And use Write-Warning "waaaah, this function will be replaced by Get-F5Pool in next Major version! Please change your workflow!"

Put them all in "deprecated_cmdlet_alias.ps1", and load it with "import-module".

Whoever is using this module doesn't have to change everything in their code, which can be done slowly...

kevsterd commented 6 years ago

I'd agree with the the thoughts above. The cmdlets aught to have the F5 or similar string within. Other projects such as NSX's Powershell tend to include the NSX name such as Get-NsxEdgeGateway etc.

Its a great peice of work here and a shame not to update it. Managment platforms tend to manage multiple vendors devices and having this would be good.

Also agree about the use of Alias and warning message....

Vacant0mens commented 5 months ago

If needed, I can work on this compatibility change. I also have a couple of other suggestions that I'd like to get into a PR anyway.