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

Changed cmdlets to functions #1848

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

Conversation

PrzemyslawKlys
Copy link
Contributor

This PR fixes the proper exposing of functions to the user. Currently, the module exposes "cmdlets" and only some of them allow for all functions to be exposed. Those are not cmdlets but functions so we should treat them as such.

Old version example on PSGallery
image

I've removed at least 2 private functions which should never be exposed to the user:

  • Remove-EmptyValue
  • Remove-NullEntriesFromHashtable

From what I see there is a bunch of comments that say it's "internal" based on functionality so:

  • Get-M365DSCComponentsForAuthenticationType
  • Test-M365DSCObjectHasProperty
  • Get-M365DSCAuthenticationMode
  • New-M365DSCCmdletDocumentation
  • Format-EXOParams
  • Get-TeamByName
  • Convert-M365DscHashtableToString
  • New-EXOSafeAttachmentRule
  • New-EXOSafeLinksRule
  • Confirm-ImportedCmdletIsAvailable
  • Set-EXOSafeAttachmentRule
  • Set-EXOSafeLinksRule
  • Compare-PSCustomObjectArrays
  • Get-M365DSCTenantNameFromParameterSet
  • Test-M365DSCParameterState
  • Confirm-M365DSCDependencies
  • Get-M365DSCTenantDomain
  • Get-M365DSCOrganization
  • New-M365DSCConnection
  • Get-SPOAdministrationUrl
  • Get-M365TenantName
  • Split-ArrayByParts
  • Invoke-M365DSCCommand
  • Get-SPOUserProfilePropertyInstance
  • ConvertTo-SPOUserProfilePropertyInstanceString
  • Get-AllSPOPackages
  • Remove-NullEntriesFromHashtable
  • Assert-M365DSCTemplate
  • Test-M365DSCDependencies
  • Remove-EmptyValue
  • Update-M365DSCExportAuthenticationResults
  • Get-M365DSCExportContentForResource

From other submodules:

  • Save-M365DSCPartialExport
  • Add-M365DSCEvent
  • Get-M365DSCResourcesByWorkloads
  • New-M365DSCStubFiles
  • Format-M365DSCString

Were commented out in the PSD1. Not sure if I have addressed everything correctly - please review and fix if necessary.

After reload only those are exposed:

Assert-M365DSCBlueprint
Export-M365DSCConfiguration
Export-M365DSCDiagnosticData
Get-M365DSCAllResources
Get-M365DSCCompiledPermissionList
Get-M365DSCTelemetryOption
Get-M365DSCWorkloadsListFromResourceNames
Import-M365DSCDependencies
Install-M365DSCDevBranch
New-M365DSCDeltaReport
New-M365DSCReportFromConfiguration
Set-M365DSCAgentCertificateConfiguration
Set-M365DSCTelemetryOption
Test-M365DSCAgent
Test-M365DSCDependenciesForNewVersions
Test-M365DSCNewVersionAvailable
Uninstall-M365DSCOutdatedDependencies
Update-M365DSCAllowedGraphScopes
Update-M365DSCDependencies
Update-M365DSCResourcesSettingsJSON

This will improve performance and not confuse users or conflict with other modules.

@PrzemyslawKlys
Copy link
Contributor Author

I see this causes some issues for testing, as you're using internal functions in an external unit tests. The workaround would be to use Import-Module with PassThru as shown https://evotec.xyz/executing-hidden-or-private-functions-from-powershell-modules/

@ykuijs
Copy link
Member

ykuijs commented May 19, 2022

@PrzemyslawKlys, I totally agree that we should be hiding as much of the functions as possible. But unfortunately, we are running into some limitations/issues/whatever you want to call them:
We are using nested modules here to store functions and these modules are auto loaded in the module manifest via the NestedModules parameter. The issue we have is that if we hide the functions that are marked as internal, we cannot call these functions from any of the DSC resources.

A good example here is the Test-M365DSCParameterState function, which is stored in the M365DSCUtil.psm1 file and which is used by all M365DSC resources. If we do not publish that function, none of the resources recognizes this function which means the Test method (where this function is used) will fail.

This is the reason that we documented the parameters as either:

  1. "Public": These functions are published and can be used by anyone. These cmdlets are documented on the microsoft365dsc.com site/
  2. "Internal": These functions are meant to be used the code in M365DSC or during development of resources. Therefore these have to be published as well, but are not documented on the microsoft365dsc.com site.
  3. "Internal, Hidden": These are helper functions that are only used by other functions in the same nested module. Therefore these functions are hidden and are not documented on the microsoft365dsc.com site.

If you have other ideas how we can resolve the above issues, that would be excellent!

@PrzemyslawKlys
Copy link
Contributor Author

For the testing Pester has what is called InModuleScope - https://pester-docs.netlify.app/docs/commands/InModuleScope so exposing functions just for tests is not necessary.

For "development" what I sometimes do with my modules is that I am exposing all of the functions/aliases during development using *, but "build" module for deployment with much more robust handling and merging of necessary modules.

Alternatively you can always call private/internal functions using & (gmo ModuleWithPrivateFunction) { PrivateFunction } (I believe InModuleScope uses that, and it's similar to what i wrote in my blog post).

Finally - I have an even more complicated process for my own modules where my development module is built out of hundred PS1 files, or even based on multiple other modules, but once I publish it to PSGallery all that data is merged into single PSM1 file with single PSD1 file, and there's no nesting or anything.

https://www.powershellgallery.com/packages/PSWriteHTML/0.0.173 -> you can see in there I have one PSD1/PSM1 and when you go to github https://github.com/EvotecIT/PSWriteHTML/blob/master/PSWriteHTML.psd1 I have defined PSSharedGoods module that is in required modules, and https://github.com/EvotecIT/PSWriteHTML/tree/master/Private many private functions that are not getting exposed - but only on the build/publish time.

I guess the easiest way is to fix Pester Tests using inmodulescope, and for that internal use (if things doesn't work between nested module - haven't tested this...) use & (gmo ModuleWithPrivateFunction) { PrivateFunction } to achieve it.

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

Successfully merging this pull request may close these issues.

3 participants