Template polish: SemVer manifest tests, init refactor, analyzer config, README split - #14
Conversation
…ception
- tests/Help.tests.ps1: rename undefined $parameterNames to $commandParameterNames
in the help-vs-code parameter existence check. The variable was never assigned
in scope, silently making the assertion always evaluate against $null.
- {{ModuleName}}/{{ModuleName}}.psm1: re-throw original exception object in the
dot-source catch block instead of throwing a new string, preserving stack
traces and inner-exception details for debugging.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Replace the single-shape Manifest.tests.ps1 dependency check with constraint- specific assertions and a SemVer comparison helper module. - tests/ManifestHelpers.psm1: new module exporting Test-VersionConstraint, with SemVer 2.0.0 prerelease ordering, .NET Version normalization, and Equal / GreaterOrEqual / LessOrEqual constraint modes. - tests/Manifest.tests.ps1: differentiate ModuleVersion (minimum), RequiredVersion (exact), and MaximumVersion (maximum) checks; accept both string and hashtable shapes for entries in requirements.psd1; detect duplicate RequiredModules entries; skip with a reason for plain-string dependencies. Preserves the existing BHBuildOutput override that points at Output/<Module>/<Version>/. Validated end-to-end against an initialized SmokeTest module: 30 passed, 0 failed, 2 -Skip'd. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- .gitattributes: mark docs/en-US/* as linguist-generated so platyPS-generated
help files don't skew GitHub language stats.
- .markdownlint-cli2.jsonc: relax MD013 in tables and code blocks, allow MD024
duplicate headings under different parents, and ignore AGENTS.md, the
generated docs/en-US/**, and the instructions/** AI-agent guides.
- docs/en-US/about_{{ModuleName}}.help.md: stub for Get-Help about_<Module>.
Initialize-Template.ps1 already renames {{ModuleName}} files in docs/en-US.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>- PSScriptAnalyzerSettings.psd1: replace the single-line @{ IncludeRules = @('*') }
with the standard structured form (IncludeDefaultRules + Include/Exclude/Rules
blocks). Includes a commented-out PSUseCompatibleSyntax/PSUseCompatibleCmdlets
scaffold for projects that want cross-version compatibility checks.
- build.depend.psd1: bump PSScriptAnalyzer from 1.24.0 to 1.25.0 to pick up
newer rule fixes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>The repo's README.md was a placeholder-laden module README, which made it look
like docs for a hypothetical {{ModuleName}} when viewed on GitHub. Split into
two files so the GitHub template page sells the template itself.
- README.md: rewritten as template documentation — what's included (build,
CI, devcontainer, instructions, test infra), quick-start with Initialize-
Template.ps1, placeholder reference table, and post-init project structure.
- README.template.md: holds the original placeholder-based module README. The
file-processing loop in Initialize-Template.ps1 still substitutes its
placeholders, then the script moves it over README.md as the final step.
- Initialize-Template.ps1:
- Rename files in docs/en-US/ that contain {{ModuleName}} (e.g.,
about_{{ModuleName}}.help.md), parallel to the Public/Private/test
Prefix-rename loop.
- After renames, replace template-facing README.md with the now-substituted
README.template.md.
- Adjust the "Next steps" message to match the new flow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>📝 WalkthroughWalkthroughAdds documentation/lint configuration, README templates and initialization behavior, upgrades analyzer deps, improves module dot-sourcing error handling, and introduces SemVer-aware manifest validation helpers plus updated tests to validate manifest vs. requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test Runner
participant Manifest as manifestRawData
participant Requirements as requirements.psd1
participant Helpers as ManifestHelpers (Test-VersionConstraint)
participant Assert as Assertion
TestRunner->>Manifest: load manifestRawData
TestRunner->>Requirements: import requirements.psd1
TestRunner->>Helpers: call Test-VersionConstraint(ManifestVersion, RequirementsVersion, Constraint)
Helpers->>Helpers: parse SemVer, compare prerelease, evaluate constraint
Helpers-->>TestRunner: result (pass/fail or throw)
TestRunner->>Assert: assert constraint results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Ports portable improvements from an upstream PowerShell module template into this repo’s template scaffolding, primarily enhancing test robustness (manifest/dependency validation and help tests), initialization behavior, and template/tooling configuration.
Changes:
- Improve test suite: fix help test variable bug; add SemVer-aware dependency constraint checks and duplicate
RequiredModulesdetection. - Update template/tooling scaffolding: add markdownlint config, about-help stub,
.gitattributes, structured PSScriptAnalyzer settings, and bump PSScriptAnalyzer dependency. - Improve initialization workflow: introduce
README.template.mdswap and rename docs placeholder files during init.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
{{ModuleName}}/{{ModuleName}}.psm1 | Adjusts dot-sourcing error handling to preserve original exception on failure. |
tests/ManifestHelpers.psm1 | Adds SemVer comparison/constraint helper used by manifest tests. |
tests/Manifest.tests.ps1 | Enhances manifest dependency validation (constraints, requirements shapes, duplicate detection). |
tests/Help.tests.ps1 | Fixes undefined variable in help parameter validation test. |
docs/en-US/about_{{ModuleName}}.help.md | Adds stub about-help markdown for the template module. |
build.depend.psd1 | Bumps PSScriptAnalyzer dependency version. |
README.template.md | Adds module-facing README template for post-init output. |
README.md | Replaces top-level README with template-facing documentation and init instructions. |
PSScriptAnalyzerSettings.psd1 | Replaces minimal analyzer settings with structured config scaffold. |
Initialize-Template.ps1 | Adds docs placeholder renaming and README swap during initialization; updates next-steps text. |
.markdownlint-cli2.jsonc | Adds markdownlint-cli2 configuration and ignores for generated/docs content. |
.gitattributes | Marks docs/en-US/* as linguist-generated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
{{ModuleName}}/{{ModuleName}}.psm1 (1)
8-9: Use a barethrowhere to preserve the original exception context.
throw $_rethrows the object, but PowerShell documentsthrowinside acatchwith no expression as the form that preserves the original execution info/line numbers. SinceWrite-Erroris non-terminating by default, this also likely emits an extra error record before the terminating rethrow. (learn.microsoft.com)♻️ Suggested adjustment
} catch { Write-Error "Unable to dot source '$($import.FullName)'" - throw $_+ throw }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @{{ModuleName}}/{{ModuleName}}.psm1 around lines 8 - 9, Replace the current rethrow in the catch block that handles dot-sourcing failures (the lines containing Write-Error "Unable to dot source '$($import.FullName)'" and throw $_) with a bare throw to preserve the original exception context and stack/line info; locate the catch around the dot-source of $import.FullName and change the rethrow expression from "throw $_" to "throw" (you may keep the Write-Error call if you still want a non-terminating error record before rethrowing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/en-US/about_`{{ModuleName}}.help.md:
- Around line 17-19: Remove the bare placeholder under the "## KEYWORDS" section
— delete the single dash line (`-`) so it doesn't render as an empty bullet, or
replace it with actual comma-separated keywords if available; locate the "##
KEYWORDS" header in the docs/en-US/about_{{ModuleName}}.help.md file and edit
the content directly to either leave the section empty (no list item) or
populate it with real keywords.
In `@README.md`:
- Around line 83-102: The markdown code fence in README.md around the project
tree is missing a language tag and triggers markdownlint MD040; update the
fenced block that starts with the lines containing "<ModuleName>/" so the
opening fence is "```text" (i.e., add the language tag "text" to that code
block) to satisfy the linter and keep the README consistent.
In `@README.template.md`:
- Around line 69-82: The fenced diagram block in README.template.md is missing a
language tag which triggers markdownlint MD040; update the opening fence for the
block that contains the "{{ModuleName}}/ ├── {{ModuleName}}/ ..." tree diagram
by adding the "text" label (i.e., change the opening ``` to ```text) so the
diagram is treated as plain text and the linter stops flagging MD040.
In `@tests/Manifest.tests.ps1`:
- Around line 203-214: When extracting the version from $requirements for
$dependencyName (in the string and hashtable branches that assign
$requirementsVersion), validate that the value is non-empty and not just
whitespace: after reading the string or .Version value, trim it and if it's
empty set $requirementsVersionSkipReason to indicate the requirements.psd1 entry
is invalid (e.g. "requirements.psd1 entry for '$dependencyName' has an empty
Version") instead of passing a blank into Test-VersionConstraint; update the
logic around the string branch and the hashtable branch (where
$requirementsVersion is assigned) to perform this guard and avoid later hard
test failures.
In `@tests/ManifestHelpers.psm1`:
- Around line 42-50: Split-SemVerString incorrectly treats build metadata (after
'+') as part of the prerelease and also casts prerelease identifiers to [long],
which mis-parses versions like "1.2.3-alpha+001" and can overflow; update the
Split-SemVerString logic to first separate build metadata by splitting the input
on '+' (store Build metadata separately), then split the left side on '-' to get
Version and Prerelease, and avoid casting prerelease identifiers to [long] (keep
them as strings or use arbitrary-size numeric comparison) so subsequent SemVer
comparisons operate on proper prerelease identifiers and ignore build metadata
per SemVer rules.
---
Nitpick comments:
In @{{ModuleName}}/{{ModuleName}}.psm1:
- Around line 8-9: Replace the current rethrow in the catch block that handles
dot-sourcing failures (the lines containing Write-Error "Unable to dot source
'$($import.FullName)'" and throw $_) with a bare throw to preserve the original
exception context and stack/line info; locate the catch around the dot-source of
$import.FullName and change the rethrow expression from "throw $_" to "throw"
(you may keep the Write-Error call if you still want a non-terminating error
record before rethrowing).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8abb7355-d4b1-48e0-83f1-9ddb948653d9
📒 Files selected for processing (12)
.gitattributes.markdownlint-cli2.jsoncInitialize-Template.ps1PSScriptAnalyzerSettings.psd1README.mdREADME.template.mdbuild.depend.psd1docs/en-US/about_{{ModuleName}}.help.mdtests/Help.tests.ps1tests/Manifest.tests.ps1tests/ManifestHelpers.psm1{{ModuleName}}/{{ModuleName}}.psm1
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- ManifestHelpers: strip SemVer build metadata in Split-SemVerString;
use [bigint] (not [long]) for numeric prerelease comparison so large
identifiers don't overflow.
- Initialize-Template: replace 'docs\en-US' and 'tests\Unit\{Public,Private}'
with forward-slash child paths so docs/test renaming works on PS7+
Linux/macOS.
- Manifest.tests: skip with a clear reason when requirements.psd1 has an
empty/whitespace Version, instead of throwing inside Test-VersionConstraint.
- {{ModuleName}}.psm1: bare 'throw' in the dot-source catch to preserve
the original ErrorRecord (was 'throw $_').
- about_{{ModuleName}}.help.md: drop trailing lone '-' that rendered as
an empty bullet under KEYWORDS.
- README.md / README.template.md: label project-tree fence as 'text'
(markdownlint MD040).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Uh oh!
There was an error while loading. Please reload this page.
The Fixed entry described the change as `throw $_` → bare `throw`, but `throw $_` was an intermediate state inside PR #14's review iterations. The actual user-visible change from main was: a string `throw` (which wrapped the original error in a fresh ErrorRecord) → bare `throw` (which preserves the original ErrorRecord and stack trace). Addresses Copilot inline review comment on PR #15. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Help.tests.ps1(undefined$parameterNames→$commandParameterNames) plus.psm1catch block now preserves the original exception viathrow $_.tests/ManifestHelpers.psm1exportsTest-VersionConstraint;Manifest.tests.ps1differentiatesRequiredVersion/ModuleVersion/MaximumVersion, accepts both string and hashtable shapes inrequirements.psd1, and detects duplicateRequiredModules..gitattributes(linguist-generatedfordocs/en-US/*),.markdownlint-cli2.jsonc, anddocs/en-US/about_{{ModuleName}}.help.mdstub.PSScriptAnalyzerSettings.psd1with the structured form (Include/Exclude/Rules + commented compat scaffold); bumpPSScriptAnalyzer1.24.0 → 1.25.0.README.md(what GitHub visitors see) and a module-facingREADME.template.md(substituted intoREADME.mdduring init); also renamedocs/en-US/{{ModuleName}}files during init.Test plan
Initialize-Template.ps1non-interactively against a copy of the repo: module folder, prefix files,docs/en-US, and README swap all renamed/populated correctly../build.ps1 -Task Test -Bootstrapagainst the initialized smoke-test module: 30 passed, 0 failed, 2-Skip'd (intentional git-tagging tests).Initialize-Template.ps1interactively and verify prompts/confirmation flow.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores