Repository navigation
Keep the module's state in module scope, not global - #19
Merged
Merged
Conversation
An unintended side effect of how the module is built, such as state it kept in a global variable where any script could read it, is not functionality the module ever offered. Correcting one is not a breaking change and needs no deprecation, so say that in the contract's definition and in Deciding hard cases. AGENTS.md now points at that definition for when a changelog entry is a BREAKING CHANGE.
Set-ScriptMessageConfigFilePath and the cached service context kept their state in global variables, so any script could read or change it, and it outlived Remove-Module. Both now live in $script: variables that the .psm1 declares, and VariablesToExport is an empty list. Callers get a supported way to read the path instead: Get-ScriptMessageConfig -ReturnConfigFilePath adds a ConfigFilePath property holding the full path of the file it read. The error for a missing path names the cmdlet to run rather than a variable. Importing the module again with -Force now clears the path, which the changelog records. Research note section 12 has the measurements of what module scope survives. The changelog's version separators also get the blank line MD022 wants, and AGENTS.md shows it in its example.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Set-ScriptMessageConfigFilePathand the cached service context kept their state in global variables ($ScriptMessage_Global_ConfigFilePathand$ScriptMessage_Global_CachedServiceContext), so any script could read or change them, and they outlivedRemove-Module. They are now module-scoped variables.Get-ScriptMessageConfig -ReturnConfigFilePathadds aConfigFilePathproperty holding the full path of the configuration file that was read. It is the supported way to read the path.Import-Module -Force, or removing and importing the module, callSet-ScriptMessageConfigFilePathagain.RELEASING.mdnow says that correcting an unintended side effect of how the module is built is not a breaking change, andAGENTS.mdpoints itsBREAKING CHANGE:rule at that definition. That is why removing the global variables is not breaking: in 1.1.1 they appear only in code and in an error message, never in the help, README, changelog, or wiki.Research_Notes/PowerShell-Language-Behavior.mdsection 12 records the measurements of what module scope survives.AGENTS.md's example shows it.Changelog: the New Parameter entry under Added, and the entry under Changed.
Release classification
Checklist
develop, ormainfor a hotfixCHANGELOG.mdhas an entry in the right section under[Unreleased](added if missing), or the change is not user-visible.\Tests\Invoke-Tests.ps1passes