Repository navigation
Refactor ImportModule commandlet #8936
Description
Refactor Import-Module part of the codebase
After speaking with Rob Holt (@rjmholt) I've learned that the Import-Module part of the codebase has gotten rather complex and needs to be refactored. However, due to numerous subtleties in how Import-Module works we need to achieve much better testing coverage of Import-Module before work can be done to refactor.
For some context the following PR is what is leading me to create this issue as I noticed some odd behavior which is described in the comments : #8758
I'm opening this meta issue to track ongoing work relating to this refactoring and hopefully it can lead to useful discussions. I will also use it to add my own insights, ideas and questions as I dive into it.
Reacted by Rob Holt
Activity
[-]Feature Request[/-][+]Refactor ImportModule commandlet[/+]rjmholt commentedon Feb 21, 2019
A few PRs in this vein:
Note an example of a regression caused by this refactoring, fixed in #8218.
pougetat commentedon Mar 20, 2019
Hey Rob Holt (@rjmholt), Ilya (@iSazonov) 😄 ,
A potential refactoring I've been thinking about has to to with
LoadModuleManifestmethod withinModuleCmdletBase.csThis method does essentially two things :
Since we already have a
TestModuleManifest.cswhich usesLoadModuleManifest+ flag disabling all the imports it might be interesting to move the checks toTestModuleManifest.cs(something along the lines ofCheckManifestProperties) and haveLoadModuleManifestcall that method before carrying on with the imports.This has the following benefits :
TestModuleManifest.csso that the property checking can be "owned" by the file implementing the associated cmdlet.ModuleCmdletBase.cssize by 25% making it more readable and relieving it of some of its responsibilities improving overall readability.This has the following potential issues :
LoadModuleManifestintertwines simple property checks with imports of related modules and therefore taking into account the previous point we could very well be introducting breaking changes by following through with that refactoring idea.Currently the method does :
TestSomeProperties => ImportModules => TestSomeMorePropertiesAnd I am thinking of :
TestAllProperties => ImportModulesHere are the steps to get to where we'd like to go if this strategy gets a green light :
One question I've had regarding the testing base in general is :
iSazonov commentedon Mar 21, 2019
adityapatwardhan commentedon Mar 21, 2019
/cc James Truher (@JamesWTruher) is working on fixing CodeCov.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 16, 2023
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you.
microsoft-github-policy-service commentedon Nov 23, 2023
This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes.