refactor(linting): extract shared boilerplate into helper functions #379
Conversation
Resolves microsoft#323 Add shared helper functions to LintingHelpers.psm1: - Test-LintingFilesExist: Common file existence check and messaging - Write-LintingHeader: Standardized header output Update Invoke-PSScriptAnalyzer.ps1 and Invoke-YamlLint.ps1 to use the shared helpers, reducing ~15 lines of duplicate boilerplate each. Note: Some unit tests may need mock scope updates (-ModuleName) due to the helper functions being called from within the module.
katriendg
left a comment
katriendg
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
The basis of this PR is looking good, but it's not yet complete. There is no function named New-LintingContext as described in the Issue.
You have added a new function but it's never called.
Can you update the references to replace Write-Host header calls with Write-LintingHeader in Invoke-YamlLint.ps1 and Invoke-PSScriptAnalyzer.ps1?
Idea: have you evaluated using the rpi-agent and point it at the issue to have it implement all requirements? Also explicitly ask the agent to help you update any of the Tests which are related to the changed files.
- Add New-LintingContext function that handles common file discovery and changes-only filtering as requested in microsoft#323 - Update Invoke-PSScriptAnalyzer.ps1 to use New-LintingContext - Update Invoke-YamlLint.ps1 to use New-LintingContext - Write-LintingHeader is now called via New-LintingContext
…der, New-LintingContext Updates LintingHelpers.Tests.ps1 with comprehensive tests for the new helper functions as requested in code review.
|
Thanks for the review! |
katriendg
left a comment
katriendg
left a comment
There was a problem hiding this comment.
Looks good now, but please make sure you update the PR description to document that New-LintingContext is the primary function that implements the refactoring. It's missing from the overview.
|
Sharing here the backlog to address the unit test mock updates #396 |
|
Description updated; maybe work on #396 later. |
|
@littleKitchen - I'll resolve the branch conflicts shortly. If you could, hold off on more work in this space as I'm trying to get your other PRs merged and some testing improvements (from my own branch) landed later today. |
sure, thanks. |
3 participants
Summary
Resolves #323
Extracts common boilerplate from linting wrapper scripts into shared helper functions.
Changes
New functions in LintingHelpers.psm1:
New-LintingContext- Primary function that creates a standardized context object for linting operations, implementing the core refactoringTest-LintingFilesExist- Handles common "no files found" pattern with standardized messagingWrite-LintingHeader- Standardized header output for linting scriptsUpdated scripts:
Invoke-PSScriptAnalyzer.ps1- UsesTest-LintingFilesExistInvoke-YamlLint.ps1- UsesTest-LintingFilesExistTesting
npm run lint:ps- Scripts execute correctlynpm run lint:yaml- Scripts execute correctly-ModuleName LintingHelpers) - tracked in [Issue]: Update mock scoping in linting tests to target LintingHelpers module #396Notes
This is a minimal refactoring that extracts the most common boilerplate while maintaining backward compatibility.