Conversation
Automates: npm run build → copy dist/ → dotnet publish --self-contained. Accepts optional RID argument (defaults to current platform). Supports win-x64, linux-x64, osx-x64, osx-arm64. Publishes a single-file executable to artifacts/publish/<RID>/.
Windows-native equivalent of build-release.sh for users who prefer PowerShell over Git Bash. Accepts -Rid parameter, defaults to win-x64.
The wwwroot/ directory is populated by build-release.sh/.ps1 from the frontend dist/ output and should not be tracked in source control.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Self-review findings: Error handling
Path safety
wwwroot/ does not exist yet (PKG-01 is open)
No issues found that require blocking changes. The trim risk is a known trade-off documented in the issue and should be validated in an integration test once PKG-01 is done. |
There was a problem hiding this comment.
Code Review
This pull request introduces unified release build scripts for both Windows (PowerShell) and Linux/macOS (Bash) environments, automating the frontend build, static file deployment, and .NET backend publishing. The .gitignore file is updated to exclude the generated wwwroot directory. Review feedback suggests enhancing the PowerShell script's cross-platform compatibility by using forward slashes or Join-Path for path construction and using wildcard matching for Windows Runtime Identifiers to support future architectures.
| $FrontendDir = Join-Path $RepoRoot "frontend\taskdeck-web" | ||
| $FrontendDist = Join-Path $FrontendDir "dist" | ||
|
|
||
| $ApiProject = Join-Path $RepoRoot "backend\src\Taskdeck.Api\Taskdeck.Api.csproj" | ||
| $Wwwroot = Join-Path $RepoRoot "backend\src\Taskdeck.Api\wwwroot" |
There was a problem hiding this comment.
For better cross-platform compatibility with PowerShell Core on Linux and macOS, it's recommended to use forward slashes as path separators in your Join-Path calls. PowerShell on Windows handles forward slashes correctly, and this change will make the script more portable without affecting its functionality on Windows.
$FrontendDir = Join-Path $RepoRoot "frontend/taskdeck-web"
$FrontendDist = Join-Path $FrontendDir "dist"
$ApiProject = Join-Path $RepoRoot "backend/src/Taskdeck.Api/Taskdeck.Api.csproj"
$Wwwroot = Join-Path $RepoRoot "backend/src/Taskdeck.Api/wwwroot"
| Get-ChildItem -Path $Wwwroot | Remove-Item -Recurse -Force | ||
| } | ||
|
|
||
| Copy-Item -Path "$FrontendDist\*" -Destination $Wwwroot -Recurse -Force |
There was a problem hiding this comment.
The hardcoded backslash in "$FrontendDist\*" can cause issues on non-Windows platforms where this script might be run using PowerShell Core. The backslash could be interpreted as an escape character. To make this more robust and platform-agnostic, construct the path using Join-Path.
Copy-Item -Path (Join-Path $FrontendDist '*') -Destination $Wwwroot -Recurse -Force
| Write-Step " RID : $Rid" | ||
| Write-Step " Artifact : $OutputDir" | ||
|
|
||
| $exeName = if ($Rid -eq "win-x64") { "Taskdeck.Api.exe" } else { "Taskdeck.Api" } |
There was a problem hiding this comment.
The check $Rid -eq "win-x64" is very specific. To make the script more future-proof for other Windows-based Runtime Identifiers (like win-arm64), it would be better to use a wildcard match. This makes the logic for determining the executable name more robust.
$exeName = if ($Rid -like "win-*") { "Taskdeck.Api.exe" } else { "Taskdeck.Api" }
Three genuine issues fixed across both build scripts: 1. Add blocking comment at WWWROOT constant: UseStaticFiles is not yet configured in PipelineConfiguration.cs (PKG-01 / #533 is the dependency). Without it the published binary silently returns 404 for all SPA routes. Comment makes the unshippable state explicit. 2. Replace rm -rf dir/* glob with rm -rf dir + mkdir -p to avoid bash glob-expansion failure on empty directories under set -euo pipefail (Git Bash on Windows passes literal /* to rm when nullglob is unset). PowerShell equivalent updated to Remove-Item + New-Item pattern. 3. Add TRIM WARNING comment before dotnet publish -p:PublishTrimmed=true in both scripts. EF Core, ASP.NET DI, System.Text.Json and SignalR are all reflection-heavy and can break silently under IL trimming.
Adversarial review findings — 3 genuine issues fixedAll three issues have been fixed directly on the branch (commit Finding 1 — CRITICAL:
|
|
Addressed all three Gemini review comments:
|
Summary
scripts/build-release.sh— cross-platform bash script that automates:npm run build→ copydist/tobackend/.../wwwroot/→dotnet publish --self-contained -p:PublishSingleFile=truescripts/build-release.ps1— PowerShell equivalent for Windows-native users (no Git Bash required)backend/src/Taskdeck.Api/wwwroot/since it is generated contentBoth scripts:
-Rid), defaulting to the current platformwin-x64,linux-x64,osx-x64,osx-arm64node,npm,dotnet) and warn on wrong Node.js versionnpm installautomatically ifnode_modulesis missingwwwroot/contents before copying freshdist/set -euo pipefail/$ErrorActionPreference = "Stop")artifacts/publish/<RID>/(already gitignored)Closes #534
Test plan
bash -n scripts/build-release.shpasses (syntax valid)frontend/taskdeck-web/dist) matches copy sourcebackend/src/Taskdeck.Api/wwwroot/) matches what PKG-01 (PKG-01: Add SPA static file serving to ASP.NET Core for self-contained deployment #533) will configure forUseStaticFilesartifacts/publish/andwwwroot/are gitignored and not tracked