Skip to content

B3.1.4#50

Merged
ziembor merged 2 commits intomainfrom
b3.1.4
Apr 12, 2026
Merged

B3.1.4#50
ziembor merged 2 commits intomainfrom
b3.1.4

Conversation

@ziembor
Copy link
Copy Markdown
Owner

@ziembor ziembor commented Apr 12, 2026

No description provided.

@ziembor ziembor merged commit 7f5067b into main Apr 12, 2026
8 checks passed
@claude
Copy link
Copy Markdown

claude bot commented Apr 12, 2026

PR Review: B3.1.4

Overall this PR is a reasonable simplification — consolidating docs and streamlining the build script reduces maintenance surface. A few things worth addressing before merging:


build-all.ps1 — Missing error handling after go build

Critical issue: After removing the try/catch block, the script no longer checks $LASTEXITCODE after go build. If the build fails, the script will still print a success message:

# Current code — no exit code check
if ($Verbose) {
    go build -v -ldflags="-s -w" -o $outputFile ./cmd/gomailtest
} else {
    go build -ldflags="-s -w" -o $outputFile ./cmd/gomailtest
}

Write-ColorOutput "  Built bin/gomailtest.exe — version $version" "Green"  # always runs!

Suggested fix:

if ($Verbose) {
    go build -v -ldflags="-s -w" -o $outputFile ./cmd/gomailtest
} else {
    go build -ldflags="-s -w" -o $outputFile ./cmd/gomailtest
}

if ($LASTEXITCODE -ne 0) {
    Write-ColorOutput "  ✗ Build failed with exit code $LASTEXITCODE" "Red"
    exit 1
}

Write-ColorOutput "  Built bin/gomailtest.exe — version $version" "Green"

build-all.ps1 — Version extraction gives poor errors on failure

The new one-liner will throw an opaque NullReferenceException if version.go doesn't exist or the pattern doesn't match:

$version = (Select-String -Path ... -Pattern ...).Matches[0].Groups[1].Value

The old explicit error-checking approach was more user-friendly. If you want to keep it concise, at minimum add a null guard:

$match = Select-String -Path (Join-Path $PSScriptRoot "internal/common/version/version.go") -Pattern 'Version = "([^"]+)"'
if (-not $match) { Write-ColorOutput "ERROR: Could not extract version from version.go" "Red"; exit 1 }
$version = $match.Matches[0].Groups[1].Value

version.go — Version jump from 3.1.1 to 3.1.4

The version jumps three patch versions (3.1.1 → 3.1.4). If 3.1.2 and 3.1.3 were used internally or in prior branches, that's fine — but worth confirming this is intentional and that the changelog entries exist for each skipped version if the project tracks those.


.github/workflows/build.ymlfail-fast: false

Good addition. This ensures a macOS or Linux build failure doesn't cancel the Windows build mid-run, giving more complete CI signal.


Documentation consolidation

The TOOLS.md consolidation is clean and the workflow changes are consistent across both Windows and Linux/macOS ZIP steps. The release notes wording improvement (** *_TOOL_README.md****TOOLS.md**) also fixes a pre-existing markdown formatting quirk.


Summary

Item Severity Status
No error check after go build High Needs fix
Opaque error on version extraction failure Low Suggested improvement
Version jump 3.1.1 → 3.1.4 Low Verify intentional
fail-fast: false Good change
Docs consolidation Good change

The build error check is the main blocker — without it, CI could report a green build when compilation actually failed.

@ziembor ziembor deleted the b3.1.4 branch April 12, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant