Website: use PowerForge pipeline --fast for -Serve#87
Conversation
|
Claude finished @PrzemyslawKlys's task —— View job Code Review AnalysisTodo List:
Review FeedbackThis PR introduces a sensible improvement to the website build script by enabling fast mode by default for local development. Here's my comprehensive analysis: ✅ Code Quality & Best PracticesStrengths:
Logic Implementation: $UseFast = ($Fast -or ($Serve -and -not $NoFast)) # Line 115This logic correctly implements the intended behavior:
🐛 Potential IssuesMinor Parameter Validation Gap: if ($Fast -and $NoFast) {
Write-Warning "Both -Fast and -NoFast specified. -Fast takes precedence."
}No Functional Bugs Detected: The core logic is sound and the parameter precedence is well-defined. ⚡ Performance ConsiderationsPositive Impact:
🔒 Security AssessmentNo Security Concerns:
🧪 Test Coverage ImplicationsAreas to Consider:
Suggested Test Scenarios: # Default behavior unchanged
./build.ps1 # Should NOT use --fast
# New fast-by-default behavior
./build.ps1 -Serve # Should use --fast
./build.ps1 -Serve -NoFast # Should NOT use --fast
# Explicit control preserved
./build.ps1 -Fast # Should use --fast
./build.ps1 -Serve -Fast # Should use --fast (redundant but valid)📋 Additional Recommendations
🎯 Overall AssessmentThis is a well-implemented enhancement that improves developer experience without breaking existing workflows. The parameter design is intuitive, the implementation is clean, and the default behavior change is conservative (only affects Recommendation: Approve ✅ The code quality is good, follows PowerShell best practices, and the feature will meaningfully improve local development iteration speed. |
There was a problem hiding this comment.
Pull request overview
Updates the website build script to default to PowerForge pipeline --fast during local -Serve runs, improving local iteration speed while keeping non-fast builds available.
Changes:
- Adds
-Fastand-NoFastswitches to control PowerForge pipeline fast mode. - Enables fast mode by default when
-Serveis used (unless-NoFastis specified). - Conditionally appends
--fastto the PowerForgepipelineinvocation.
| @@ -103,16 +112,25 @@ function Assert-SiteOutput { | |||
| } | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
-Fast and -NoFast can both be provided at the same time; with the current expression, -Fast always wins and -NoFast is ignored. Consider adding explicit parameter validation (or precedence rules) so the behavior is unambiguous (e.g., throw when both are set, and optionally warn/throw when -NoFast is used without -Serve).
| try { | |
| try { | |
| if ($Fast -and $NoFast) { | |
| throw "Invalid parameter combination: -Fast and -NoFast cannot be used together. Choose one." | |
| } | |
| if ($NoFast -and -not $Serve) { | |
| Write-Warning "-NoFast has no effect when -Serve is not specified. Fast mode is only implied when serving." | |
| } |
| if ($UseFast) { | ||
| & $PowerForge @PowerForgeArgs pipeline --config pipeline.json --profile --fast | ||
| } else { | ||
| & $PowerForge @PowerForgeArgs pipeline --config pipeline.json --profile | ||
| } | ||
| if ($LASTEXITCODE -ne 0) { throw "Build failed (exit code $LASTEXITCODE)" } | ||
| Assert-SiteOutput -SiteRoot (Join-Path $PSScriptRoot '_site') | ||
| Write-Host "Starting dev server on http://localhost:$Port ..." -ForegroundColor Cyan | ||
| & $PowerForge @PowerForgeArgs serve --path _site --port $Port | ||
| } else { | ||
| Write-Host 'Building website...' -ForegroundColor Cyan | ||
| & $PowerForge @PowerForgeArgs pipeline --config pipeline.json --profile | ||
| if ($UseFast) { | ||
| & $PowerForge @PowerForgeArgs pipeline --config pipeline.json --profile --fast | ||
| } else { | ||
| & $PowerForge @PowerForgeArgs pipeline --config pipeline.json --profile | ||
| } |
There was a problem hiding this comment.
The pipeline invocation logic is duplicated in both the -Serve and non-serve branches (including the if ($UseFast) block). Consider building a single argument list once (adding --fast conditionally) and calling powerforge once before branching into serve vs. "build complete" output; this reduces duplication and the risk of the branches drifting.
| .PARAMETER Fast | ||
| Run the pipeline with `--fast` (recommended for local iteration). | ||
| When `-Serve` is used, fast mode is enabled by default unless `-NoFast` is set. | ||
|
|
||
| .PARAMETER NoFast | ||
| Disable fast mode when `-Serve` is used. | ||
|
|
There was a problem hiding this comment.
The comment-based help documents the new -Fast/-NoFast switches here, but the .EXAMPLE section below doesn’t include any usage showing how to disable fast mode for -Serve or how to force fast mode without -Serve. Consider adding one or two examples so the new behavior is easier to discover via Get-Help.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #87 +/- ##
==========================================
+ Coverage 59.74% 59.76% +0.01%
==========================================
Files 448 448
Lines 57814 57814
Branches 16563 16563
==========================================
+ Hits 34540 34551 +11
+ Misses 17567 17564 -3
+ Partials 5707 5699 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Use PowerForge pipeline --fast by default when running Website\build.ps1 -Serve (can be disabled via -NoFast or overridden via -Fast). This keeps local dev iterations quick while CI/release can still run full optimize/audit.