-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Include Module Version in error messages when module is not found #20144
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
Include Module Version in error messages when module is not found #20144
Conversation
What if there are some modules? |
|
I agree with @iSazonov that the output, although technically correct, isn't the most user friendly particularly those who simply got a script to run. It may be better to have a new resource string in a resx file added and specifically have the module name and module version instead of the hashtable syntax. |
Thanks @SteveL-MSFT @iSazonov. I thought the same initially but wasn't sure what format we should include. The hashtable string looks ok for one module but for many it gets quite hard to read. Could we do something like Or should we have a full resx message like Happy to do whatever is preferred and most user friendly 😄. |
|
@ArmaanMcleod my initial thinking is the latter: |
|
Since the directive has 3 parameters ModuleVersion, MaximumVersion , RequiredVersion (https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_requires?view=powershell-7.3#-modules-module-name--hashtable) my first question is - are two first error messages correct or we should fix them too? I think all three messages should have the same format (not one resx string I mean).
|
This but if is a max version, not required version then the message should state along lines of So would need both in for full clarity. |
src/System.Management.Automation/engine/Modules/ModuleSpecification.cs
Outdated
Show resolved
Hide resolved
iSazonov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor proposal.
src/System.Management.Automation/engine/Modules/ModuleSpecification.cs
Outdated
Show resolved
Hide resolved
SteveL-MSFT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great change! Thanks!
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |

PR Summary
Fixes #20142
PR Context
Added more user friendly
#requireserror message when a module cannot be loaded, instead of just the module name.If you had a required module included like:
#Requires -Modules @{ ModuleName = 'CosmosDB'; RequiredVersion = '4.6.0'}Previous Behaviour
Just shows module name.
Current Behaviour
Includes full RESX message with module name and version.
The new RESX messages are like below for each version type and if no version is specified
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).