Include all batteries-included modules in IdLE package#127
Conversation
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This pull request attempts to fix a bug where the IdLE package published to PowerShell Gallery only contained 2 of 8 documented modules. However, the implementation has a critical flaw that breaks the documented "batteries-included but not auto-imported" behavior for optional modules.
Changes:
- Updated
tools/New-IdleModulePackage.ps1to include 6 additional modules in the default package - Updated
.github/workflows/release.ymlversion validation to check all 8 packaged modules
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/New-IdleModulePackage.ps1 | Added 6 modules to default $NestedModuleNames parameter, which causes them to be both included in the package AND auto-imported |
| .github/workflows/release.yml | Added 6 module manifests to version validation list (appropriate for ensuring version consistency) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
blindzero
left a comment
There was a problem hiding this comment.
I want to provide a package that has all submodules on board when users install by Install-Module -Name IdLE. But auto-import behavior should not change. They should be available for manual Import-Module -Name Idle.Provider.AD as an example, although this provider is not automatically imported when performing Import-Module.
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
…uplication Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Fixed in 608ff60 and 6276b29. The package now includes all 8 modules physically (can be installed via |
Summary
IdLE package published to PowerShell Gallery only contained
IdLE.CoreandIdLE.Steps.Common. Users installing viaInstall-Module -Name IdLEreceived an incomplete package missing 6 additional modules documented indocs/use/installation.md.Motivation
Package manifest hardcoded defaults to 2 of 8 modules. Users couldn't access provider and step modules without separate installations, contradicting documented behavior. Additionally, the original implementation would have auto-imported all modules, breaking the documented "non-blocking guarantee" for optional providers with external dependencies.
Type of Change
Changes
$IncludeModuleNamesparameter totools/New-IdleModulePackage.ps1to control which modules are physically copied into the package (defaults to all 8 batteries-included modules)$NestedModuleNamesparameter to control which modules are auto-imported when users runImport-Module IdLE(remains baseline 2 modules:IdLE.CoreandIdLE.Steps.Common)$IncludeModuleNamesfor copying modules into package and$NestedModuleNamesfor updating manifest NestedModules array.github/workflows/release.ymlversion validation to check all packaged modulesThis approach ensures all modules are available in the package for manual import (e.g.,
Import-Module IdLE.Provider.AD) while preserving the non-blocking guarantee thatImport-Module IdLEsucceeds without external dependencies.Note:
IdLE.Provider.Mockremains published separately for PowerShell Gallery discoverability.Testing
How to test & review
Build package locally and verify all 8 modules present:
Import and verify only baseline modules are auto-imported:
Verify optional modules can be manually imported:
Checklist
Related Issues
Addresses packaging discrepancy where published module lacked documented batteries-included modules while preserving non-blocking guarantee for baseline imports.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.