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

Rename internal functions to be singular to align with PowerShell best practice; Add additional debug logging for some internal exceptions #1295

Merged
merged 7 commits into from
Jun 2, 2024

Conversation

mdaneri
Copy link
Contributor

@mdaneri mdaneri commented May 10, 2024

Rename any internal function with a plural noun to a singular one
feature: #1271

Addressed the following ScriptAnalyzer warnings

  • PSAvoidUsingCmdletAliases
  • PSAvoidUsingWriteHost
  • PSAvoidUsingInvokeExpression

Suppress PSPossibleIncorrectComparisonWithNull for function New-PodeAutoRestartServer

Renamed the following internal functions:

  • Read-PodeWebExceptionDetails -> Read-PodeWebExceptionDetail
  • Test-PodeAuthUserGroups -> Test-PodeAuthUserGroup
  • Get-PodeAuthADGroups -> Get-PodeAuthADGroup
  • Get-PodeModuleDependencies -> Get-PodeModuleDependency
  • Get-PodeModuleDetails -> Get-PodeModuleInfo
  • Get-PodeDefaultSslProtocols -> Get-PodeDefaultSslProtocol
  • New-PodeRunspacePools -> New-PodeRunspacePool
  • Test-PodeEndpoints -> Test-PodeEndpoint`
  • Close-PodeRunspacePools -> Close-PodeRunspacePool
  • Set-PodeOutputVariables -> Set-PodeOutputVariable
  • Get-PodeCronFields -> Get-PodeCronField
  • Get-PodeCronFieldConstraints -> Get-PodeCronFieldConstraint
  • Get-PodeCronFieldAliases -> Get-PodeCronFieldAlias
  • ConvertFrom-PodeCronExpressions -> ConvertFrom-PodeCronExpression
  • Get-PodeRandomBytes -> Get-PodeRandomByte
  • Find-PodeEndpoints -> Find-PodeEndpointInternal
  • Test-PodeEndpoints -> Test-PodeEndpoint
  • Get-PodeErrorLoggingLevels -> Get-PodeErrorLoggingLevel
  • Test-PodeLoggerBatches -> Test-PodeLoggerBatch
  • Update-PodeServerRequestMetrics -> Update-PodeServerRequestMetric
  • Update-PodeServerSignalMetrics -> Update-PodeServerSignalMetric
  • Update-PodeRouteSlashes -> Update-PodeRouteSlash
  • Resolve-PodePlaceholders -> Resolve-PodePlaceholder
  • Complete-PodeInternalSchedules -> Complete-PodeInternalSchedule
  • Find-PodeScopedVariableUsingVariableValues -> Find-PodeScopedVariableUsingVariableValue
  • Get-PodeScopedVariableUsingVariables -> Get-PodeScopedVariableUsingVariable
  • Convert-PodeScopedVariableUsingVariables -> Convert-PodeScopedVariableUsingVariable
  • Find-PodeScopedVariableUsingVariableValues -> Find-PodeScopedVariableUsingVariableValue
  • Unregister-PodeSecretVaults -> Unregister-PodeSecretVaultsInternal
  • Clear-PodeHashtableInnerKeys -> Clear-PodeHashtableInnerKey

Exceptions

Added body to any Execptions without any code

catch [<_exception_>] {
     $_ | Write-PodeErrorLog -Level Debug
}

src/Private/Context.ps1 Outdated Show resolved Hide resolved
src/Private/Context.ps1 Outdated Show resolved Hide resolved
src/Private/Context.ps1 Outdated Show resolved Hide resolved
src/Private/Context.ps1 Outdated Show resolved Hide resolved
src/Private/Endpoints.ps1 Outdated Show resolved Hide resolved
src/Private/Helpers.ps1 Outdated Show resolved Hide resolved
src/Private/Helpers.ps1 Outdated Show resolved Hide resolved
src/Private/Timers.ps1 Outdated Show resolved Hide resolved
src/Private/Serverless.ps1 Outdated Show resolved Hide resolved
src/Private/Serverless.ps1 Outdated Show resolved Hide resolved
@mdaneri
Copy link
Contributor Author

mdaneri commented May 13, 2024

The function Add-PodeSecurityHeader is not name compliant and should be named Set-PodeSecurityHeader
but I know that it's a tricky change. Maybe we can create a Set-PodeSecurityHeader alias in the meantime

@Badgerati
Copy link
Owner

We need to be careful we're not making too many aliases here for pubic functions, otherwise it will start to cause confusion when people ask for help. For any public functions that need a rename, I'd say it would be better targeted at a v3.0 where we can write a migration guide for them. (I already have ideas for a v3, so might start shortlisting them soon - a lot of them are fixes for better best practices)

I was OK with OpenAPI as there were 1 or 2 functions that didn't conform to the "PodeOA" tag. But for some that might have a slightly wrong verb, or be plural, in the public functions, I'd say leave them for now - otherwise we'll have a flood of aliases, which borders on bad practice.

src/Private/Security.ps1 Outdated Show resolved Hide resolved
src/Private/Middleware.ps1 Outdated Show resolved Hide resolved
src/Private/Security.ps1 Outdated Show resolved Hide resolved
src/Private/Security.ps1 Outdated Show resolved Hide resolved
src/Private/Security.ps1 Outdated Show resolved Hide resolved
src/Private/Security.ps1 Outdated Show resolved Hide resolved
 	modified:   src/Private/Middleware.ps1
 	modified:   src/Private/Security.ps1
	modified:   src/Private/Serverless.ps1
	modified:   src/Private/Timers.ps1
src/Private/Middleware.ps1 Outdated Show resolved Hide resolved
src/Private/Middleware.ps1 Outdated Show resolved Hide resolved
src/Private/ScopedVariables.ps1 Outdated Show resolved Hide resolved
src/Private/Cryptography.ps1 Outdated Show resolved Hide resolved
src/Public/Authentication.ps1 Outdated Show resolved Hide resolved
src/Public/Authentication.ps1 Outdated Show resolved Hide resolved
src/Private/UnusedHelper.ps1 Outdated Show resolved Hide resolved
@Badgerati Badgerati added this to the 2.11.0 milestone May 30, 2024
@Badgerati Badgerati linked an issue May 30, 2024 that may be closed by this pull request
@Badgerati Badgerati changed the title Rename any internal function with a plural noun to a singular one Rename internal functions to be singular to align with PowerShell best practice; Add additional debug logging for some internal exceptions May 30, 2024
Copy link
Owner

@Badgerati Badgerati left a comment

Choose a reason for hiding this comment

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

I'm happy to merge if no other commits are needed?

@mdaneri
Copy link
Contributor Author

mdaneri commented Jun 2, 2024

Good to go
I'm going to open another bug for the removal of the global variable.
You showed me how to do it😏

@Badgerati Badgerati merged commit 3618f57 into Badgerati:develop Jun 2, 2024
16 checks passed
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.

Rename any internal function with a plural nouns to a singular one
2 participants