-
Notifications
You must be signed in to change notification settings - Fork 33
refactor(application): wrap execution with try blocks, ensure proper … #296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WilliamBerryiii
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check out my comments, address them and then I'll kick off the build? Thanks again for the contribution!
…nged error handling to Write-Error on Get-VerifiedDownload
|
I have fixed the two errors. Thank you so much for giving me the opportunity to contribute! Let me know if there are any problems. |
|
@microsoft-github-policy-service agree |
|
@Richard-Jang - I'll resolve your branch conflicts shortly (I made some heavy changes today that will need careful resolution), so your contribution is included in the 2.0 release 😁😁😁 |
|
Hi @Richard-Jang! 👋 There's a merge conflict in Steps to resolve# 1. Fetch and merge main into your branch
git fetch upstream # or 'origin' if you cloned microsoft/hve-core directly
git merge upstream/main
# 2. You'll see a conflict in scripts/linting/Validate-MarkdownFrontmatter.ps1The conflictThe conflict is in the Replace the entire conflicted section (from #region Main Execution
try {
if ($MyInvocation.InvocationName -ne '.') {
if ($ChangedFilesOnly) {
$result = Test-FrontmatterValidation -ChangedFilesOnly -BaseBranch $BaseBranch -ExcludePaths $ExcludePaths -WarningsAsErrors:$WarningsAsErrors -EnableSchemaValidation:$EnableSchemaValidation -SkipFooterValidation:$SkipFooterValidation
}
elseif ($Files.Count -gt 0) {
$result = Test-FrontmatterValidation -Files $Files -ExcludePaths $ExcludePaths -WarningsAsErrors:$WarningsAsErrors -EnableSchemaValidation:$EnableSchemaValidation -SkipFooterValidation:$SkipFooterValidation
}
else {
$result = Test-FrontmatterValidation -Paths $Paths -ExcludePaths $ExcludePaths -WarningsAsErrors:$WarningsAsErrors -EnableSchemaValidation:$EnableSchemaValidation -SkipFooterValidation:$SkipFooterValidation
}
# Normalize result: if pipeline output produced an array, extract the ValidationSummary object
# PowerShell functions can inadvertently output multiple objects; take the last (the return value)
if ($result -is [System.Array]) {
$result = $result | Where-Object { $null -ne $_ -and $_.GetType().GetMethod('GetExitCode') } | Select-Object -Last 1
}
# In ChangedFilesOnly mode with no changed files, TotalFiles=0 is a successful no-op
if ($ChangedFilesOnly -and $null -ne $result -and $result.TotalFiles -eq 0) {
Write-Host "✅ No changed markdown files to validate - success!" -ForegroundColor Green
exit 0
}
# Validate result object before calling GetExitCode to prevent method invocation errors
# PowerShell class methods are compiled to .NET type metadata, not stored in PSObject.Methods (ETS only)
if ($null -eq $result -or $null -eq $result.GetType().GetMethod('GetExitCode')) {
$resultTypeName = if ($null -eq $result) { '<null>' } else { $result.GetType().FullName }
Write-Host "Validation did not produce a usable result object (type: $resultTypeName). Exiting with code 1."
exit 1
}
$exitCode = $result.GetExitCode($WarningsAsErrors)
if ($exitCode -ne 0) {
exit $exitCode
}
else {
Write-Host "✅ All frontmatter validation checks passed!" -ForegroundColor Green
exit 0
}
}
}
catch {
Write-Error "Validate Markdown Frontmatter failed: $($_.Exception.Message)"
if ($env:GITHUB_ACTIONS -eq 'true') {
Write-Output "::error::$($_.Exception.Message)"
}
exit 1
}
#endregionFinish up# 3. Stage the resolved file
git add scripts/linting/Validate-MarkdownFrontmatter.ps1
# 4. Complete the merge
git commit -m "merge: integrate upstream/main changes"
# 5. Push to your fork
git push origin mainThis combines your try/catch wrapper with the result normalization logic from main. Let me know if you have any questions! |
|
Hello! I've merged the changes. Let me know if there are any issues. |
- Remove else blocks that exit on dot-source, blocking test execution - Remove invalid -Level Error parameter from Write-Error cmdlet Affected files: - scripts/lib/Get-VerifiedDownload.ps1 - scripts/extension/Package-Extension.ps1 - scripts/extension/Prepare-Extension.ps1 - scripts/dev-tools/Generate-PrReference.ps1 Co-authored-by: Richard Jang <[email protected]>
Remove else block that exits on dot-source, blocking test execution. This completes the fix for PR microsoft#296 CI failures. Co-authored-by: Richard Jang <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
==========================================
- Coverage 38.80% 38.30% -0.51%
==========================================
Files 15 15
Lines 2814 2864 +50
==========================================
+ Hits 1092 1097 +5
- Misses 1722 1767 +45
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Hey @Richard-Jang! 👋 Thank you so much for this contribution! Adding proper error handling and exit codes across our scripts is exactly the kind of foundational improvement that makes the codebase more robust and easier to maintain. This is great work, especially for a first-time contribution to the project. I pushed a couple of small fixes to get CI green (the dot-source guards were preventing our Pester tests from loading the functions, and there was a minor PowerShell parameter issue), but the core of your work is solid and valuable. If you enjoyed contributing here and want to continue, I'd love to point you toward a couple other repos where your skills would be welcome:
Both repos have Thanks again for helping make hve-core better. 🎉 |
🤖 I have created a release *beep* *boop* --- ## [2.0.0](hve-core-v1.1.0...hve-core-v2.0.0) (2026-01-28) ### ⚠ BREAKING CHANGES * **agents:** add Task Reviewer and expand RPI to 4-phase workflow ([#277](#277)) ### ✨ Features * **agents:** add hve-core-installer agent to extension package ([#297](#297)) ([c0e48c6](c0e48c6)) * **agents:** add Task Reviewer and expand RPI to 4-phase workflow ([#277](#277)) ([ae76cab](ae76cab)) * **build:** add code coverage reporting to Pester workflow ([#230](#230)) ([a34822a](a34822a)) * **docs:** add GOVERNANCE.md for OSSF Silver Badge compliance ([#235](#235)) ([b0e752c](b0e752c)) * **docs:** add ROADMAP.md for OSSF Silver badge compliance ([#238](#238)) ([4a41c16](4a41c16)) * **mcp:** add MCP server configuration guidance and installer enhancements ([#225](#225)) ([0bce418](0bce418)) * **scripts:** add YAML linting with actionlint ([#234](#234)) ([d9301f9](d9301f9)) * **security:** add OpenSSF Scorecard workflow and badge ([#271](#271)) ([7c6d788](7c6d788)) * **skills:** add video-to-gif conversion skill with FFmpeg two-pass optimization ([#247](#247)) ([8d65c42](8d65c42)) * **tests:** add Pester tests for LintingHelpers and Validate-MarkdownFrontmatter ([#197](#197), [#198](#198)) ([#205](#205)) ([51ae563](51ae563)) ### 🐛 Bug Fixes * **build:** detect table formatting changes via git diff ([#261](#261)) ([985eee0](985eee0)) * **build:** disable MD024 lint rule in CHANGELOG for release-please ([#220](#220)) ([971df94](971df94)) * **build:** quote shell variables and group redirects in workflow files ([#299](#299)) ([3372509](3372509)) * **build:** resolve scorecard badge and workflow security issues ([#301](#301)) ([aeaed13](aeaed13)) * **extension:** remove frontmatter from README and exclude from markdown linting ([#223](#223)) ([4272529](4272529)) * **instructions:** quote applyTo glob pattern for YAML compatibility ([#216](#216)) ([085199c](085199c)) * **scripts:** add FooterExcludePaths parameter to frontmatter validation ([#334](#334)) ([64db98d](64db98d)) * **scripts:** add GHSA word and logs/ exclusion to cspell config ([#214](#214)) ([5c99b3f](5c99b3f)) * **scripts:** correct type assertions in Invoke-YamlLint.Tests.ps1 ([#332](#332)) ([af7050d](af7050d)) * **scripts:** eliminate false positives in dependency pinning npm pattern ([#273](#273)) ([ccbdfa3](ccbdfa3)) * **security:** add artifact attestation for signed releases ([#257](#257)) ([c52d6e2](c52d6e2)) * standardize markdown footers and complete frontmatter ([#217](#217)) ([b4e7556](b4e7556)) ### 📚 Documentation * add OpenSSF Best Practices Passing badge to README ([#239](#239)) ([91bc529](91bc529)) * **architecture:** add architecture documentation and value proposition ([#252](#252)) ([0e4b02f](0e4b02f)) * **contributing:** add testing requirements for OSSF compliance ([#254](#254)) ([4db1a18](4db1a18)) * **docs:** add enterprise status badges to README header ([#270](#270)) ([ccb68a4](ccb68a4)) * **security:** add security assurance case and threat model for OSSF Silver ([#259](#259)) ([a390e26](a390e26)) ### ♻️ Refactoring * **application:** wrap execution with try blocks, ensure proper … ([#296](#296)) ([35c4417](35c4417)) * **scripts:** extract frontmatter validation to testable module ([#293](#293)) ([4e8707e](4e8707e)) * **scripts:** extract pure functions for Pester testability ([#221](#221)) ([d40e742](d40e742)) ### 🔧 Maintenance * **deps-dev:** bump cspell from 9.4.0 to 9.6.0 in the npm-dependencies group ([#208](#208)) ([855914b](855914b)) * **deps-dev:** bump cspell from 9.6.0 to 9.6.1 in the npm-dependencies group ([#294](#294)) ([1e45ad6](1e45ad6)) * **deps:** bump actions/setup-node from 6.1.0 to 6.2.0 in the github-actions group ([#209](#209)) ([c4c69e2](c4c69e2)) * **deps:** bump the github-actions group with 4 updates ([#295](#295)) ([d8337b8](d8337b8)) * remove step-security/harden-runner from workflows ([#246](#246)) ([c5708d8](c5708d8)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: hve-core-release-please[bot] <254602402+hve-core-release-please[bot]@users.noreply.github.com>
…exit codes (#289)
Pull Request
Description
Related Issue(s)
Fixes #289
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)Other:
.ps1,.sh,.py)Sample Prompts (for AI Artifact Contributions)
User Request:
Execution Flow:
Output Artifacts:
Success Indicators:
For detailed contribution requirements, see:
Testing
Checklist
Required Checks
AI Artifact Contributions
/prompt-analyzeto review contributionprompt-builderreviewRequired Automated Checks
The following validation commands must pass before merging:
npm run lint:mdnpm run spell-checknpm run lint:frontmatternpm run lint:md-linksnpm run lint:psSecurity Considerations
Additional Notes