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

Please publish latest version on powershellgallery #260

Closed
fredericrous opened this issue Feb 27, 2016 · 9 comments
Closed

Please publish latest version on powershellgallery #260

fredericrous opened this issue Feb 27, 2016 · 9 comments

Comments

@fredericrous
Copy link

#224 is still reproducible.

@dahlbyk
Copy link
Owner

dahlbyk commented Mar 10, 2016

I just pushed a new version, but got a warning that suggests #242 may not have been sufficient. Give it a try?

WARNING: This module 'C:\Users\Keith\AppData\Local\Temp\1744033178\posh-git\posh-git.psd1' has exported functions. As a best practice, include exported functions in the module manifest file(.psd1). You can run Update-ModuleManifest -FunctionsToExport to update the manifest with ExportedFunctions field.

@theaquamarine
Copy link
Collaborator

It looks like Test-ModuleManifest doesn't pick up the exported functions or aliases properly (except Invoke-NullCoalescing, oddly), but actually importing the module processes everything properly.

Moving exported functions from Export-ModuleMember to being directly declared in the manifest causes them to appear properly with Test-ModuleManifest so I'd guess that would fix the warning.

@lzybkr
Copy link
Collaborator

lzybkr commented Mar 10, 2016

The warning message should be improved. Using wildcards results in poor performance on V3 and beyond when using module auto-loading (so could be any command). PowerShell has to analyze the script/dll/whatever when you use wildcards, but if you avoid wildcards, PowerShell skips that potentially slow work.

So best practice is to use:

FunctionsToExport = @('Get-Thing1', 'Get-Thing2')
# no aliases to export, be explicit, don't omit this
AliasesToExport = @()
# no cmdlets to export, be explicit, don't omit this
CmdletsToExport = @()

Instead of:

# Don't use wildcards
FunctionsToExport = "*"
# Don't omit any of the *ToExport entries
## CmdletsToExport = "*"
# Don't use $null, it's handled just like it was omitted
AliasesToExport = $null

@dahlbyk
Copy link
Owner

dahlbyk commented Mar 11, 2016

So can we drop

posh-git/posh-git.psm1

Lines 30 to 48 in 5f8e878

Export-ModuleMember `
-Alias @(
'??') `
-Function @(
'Invoke-NullCoalescing',
'Write-GitStatus',
'Write-Prompt',
'Get-GitStatus',
'Enable-GitColors',
'Get-GitDirectory',
'TabExpansion',
'Get-AliasPattern',
'Get-SshAgent',
'Start-SshAgent',
'Stop-SshAgent',
'Add-SshKey',
'Get-SshPath',
'Update-AllBranches',
'tgit')
altogether or do we need to maintain the export list in two places?

@lzybkr
Copy link
Collaborator

lzybkr commented Mar 11, 2016

By default, all functions, cmdlets, and aliases are exported, so if that's what you're doing, it's not necessary. But if you have have internal functions or aliases, then maintaining the list in both places is unfortunately necessary for the best performance.

theaquamarine added a commit to theaquamarine/posh-git that referenced this issue Mar 11, 2016
Avoiding wildcards in the manifest improves performance and placates
tests. See discussion in dahlbyk#260.
@dahlbyk
Copy link
Owner

dahlbyk commented Mar 11, 2016

By default, all functions, cmdlets, and aliases are exported, so if that's what you're doing, it's not necessary.

Does this include exporting function private:Do-Something? If not, we could make a sweep through to privatize (lol) anything that's not be exported.

@lzybkr
Copy link
Collaborator

lzybkr commented Mar 12, 2016

I had a feeling private wouldn't have any effect, but had to test to be sure. And indeed, private functions get exported. That is probably an oversight, but it doesn't help you if it gets fixed, so you should stick with Export-ModuleMember.

@dahlbyk
Copy link
Owner

dahlbyk commented Dec 29, 2016

I'd like to clean up the PS Gallery versioning process as part of the push to 1.0: #328

@rkeithhill
Copy link
Collaborator

I'm closing this since the latest version is posted to the PSGallery. And the current posh-git.psd1 no longer uses *. In fact, the only warning it generates is for using ModuleToProcess instead of RootModule.

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

No branches or pull requests

5 participants