feat(scripts): add folder-consistency check and standardize WARN outp…#1350
feat(scripts): add folder-consistency check and standardize WARN outp…#1350mariekekortsmit wants to merge 2 commits intomicrosoft:mainfrom
Conversation
…ut in collection validation - add collection-id to folder name consistency check with exemptions for shared, hve-core, and hve-core-all - replace Write-Warning calls with Write-Host WARN pattern for consistent output - normalize WARN prefix spacing across all advisory messages - add folder-consistency Pester tests with positive and negative Write-Host mock assertions ✨ - Generated by Copilot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1350 +/- ##
=======================================
Coverage 87.65% 87.65%
=======================================
Files 61 61
Lines 9323 9334 +11
=======================================
+ Hits 8172 8182 +10
- Misses 1151 1152 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
katriendg
left a comment
There was a problem hiding this comment.
Thanks @mariekekortsmit for your contribution!
As I reviewed the current collections and what we are trying to achieve, we want to allow for some of the intentional cross-collection bundling. We want to catch folder names not matching collection names, but allow cross-bundling.
Could you update the check against all known collection IDs derived from the *.collection.yml filenames in collections folder. This catches real stale folders (like the original code-review/ → coding-standards/ drift from #1208) while allowing intentional cross-collection bundling.
Suggested fix:
- Build a
$knownCollectionIdsset from all manifest filenames before the per-file loop. - Change the condition from
$folderName -ne $idto-not $knownCollectionIds.ContainsKey($folderName). - Update the warning message to: "folder does not match any known collection ID".
You will probably just end up with one warning about a skill for playwright which we are aware of.
Thanks!
So that means the relation should be bidirectional? All items under a specific prefix folder (e.g. I only understood a one way relation, that explains our difference. |
|
@mariekekortsmit The warning with the playwright is correct because it is missing in a collection experimental. The issue with the current approach is it will also warn for an item from Maybe in future we will want to warn about cross-bundling, but the current version contains several which we accept and don't want to warn about. Hope that makes sense? |
Maybe I'm confused because of the word "cross-bundling" and what that means exactly so And with cross-bundling you mean, there is no check needed to verify that every item named in In the issue, one of the acceptance criteria is: If my understanding still does not match the idea, maybe we can have a quick call to figure out where the misunderstanding is. |
I apologize for the confusion, the initial issue had the aim to also warn about cross-bundling. With cross-bundling we mean that an item from a collection |
Description
Add a collection-id to folder name consistency check in
Validate-Collections.ps1that warns when a manifest item's folder doesn't match the collection id. Exemptsshared/,hve-core/folders and thehve-core-allcollection. Also standardizes allWrite-Warningcalls to theWrite-Host WARNpattern used throughout the script and normalizes WARN prefix spacing.Related Issue(s)
Closes #1209
Type of Change
Select all that apply:
Code & Documentation:
Infrastructure & Configuration:
AI Artifacts:
prompt-builderagent and addressed all feedback.github/instructions/*.instructions.md).github/prompts/*.prompt.md).github/agents/*.agent.md).github/skills/*/SKILL.md)Other:
.ps1,.sh,.py)Testing
npm run test:ps -- -TestPath "scripts/tests/collections/Validate-Collections.Tests.ps1"hve-core/exemption,shared/exemption,hve-core-allskip, and duplicate WARN output assertionMock Write-Host {}withShould -Invoke/Should -Not -InvokeandParameterFiltermatchingWARN collectionanchorChecklist
Required Checks
Required Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run validate:skillsnpm run lint:md-linksnpm run lint:psnpm run plugin:generatenpm run docs:testSecurity Considerations
Additional Notes
Write-Warningwas the outlier pattern;Write-Host WARNis the established convention in this script per maintainer direction