B3.1.6

[ziembor]

No description provided.

[claude]

Code Review — PR #51 (B3.1.6)

Note: This PR has already been merged; leaving this for the record and any follow-up work.

Summary

This PR consolidates documentation files into TOOLS.md, simplifies build-all.ps1, and adds fail-fast: false to the CI matrix. The intent is good but there are a few issues worth addressing.

---

Issues

1. Missing ChangeLog/3.1.6.md

version.go was bumped to 3.1.6, but only ChangeLog/3.1.5.md was added — not 3.1.6.md. The comment at the top of version.go explicitly says:

// 2. Create a changelog entry in ChangeLog/{version}.md

The released version is 3.1.6 with no changelog entry. A ChangeLog/3.1.6.md should be created to document what actually shipped.

---

2. ChangeLog/3.1.5.md content is not user-facing

## Added
- fighting with build on GitHub action

This reads like a personal development note, not a changelog entry. Prior entries (e.g. 3.1.0.md, 3.1.1.md) are descriptive and user-oriented. Consider something like:

## Changed
- Build script (`build-all.ps1`) simplified; removed `-SkipTests` flag
- Release archives now bundle `TOOLS.md` instead of individual `*_TOOL_README.md` files
- Added `fail-fast: false` to CI release matrix so all platform builds complete even on partial failure

---

3. build-all.ps1 — no build failure check

The original script checked $LASTEXITCODE after go build and surfaced a clear error. The simplified version does not:

go build -ldflags="-s -w" -o $outputFile ./cmd/gomailtest
# ← no $LASTEXITCODE check here

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

If go build fails, the script still prints a success message and exits with code 0, masking the failure. A minimal guard would be:

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

---

4. build-all.ps1 — version extraction has no error handling

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

If version.go is missing or the regex doesn't match, .Matches[0] will throw a NullPointerException with an unhelpful message. The original had explicit checks with clear error messages. A null-guard would help:

$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

---

Positives

• fail-fast: false on the build matrix is the right call for release jobs — ensures all platform artifacts are produced even if one runner fails.
• Documentation consolidation (TOOLS.md replacing individual *_TOOL_README.md files) is a clean simplification for distribution.
• The New-Item -Force one-liner for bin/ is more idiomatic PowerShell than the previous Test-Path guard.