Conversation
PR Review: v3.1.6 — build fixes, doc update, changelog cleanupOverall this is a clean, well-scoped PR. A few observations below. build-all.ps1Both additions are solid: Null-check for version extraction — the previous one-liner would throw a cryptic NullReferenceException if
Minor note: the failure message just prints PR description vs. actual diff — discrepancyThe first bullet in the description says:
However, If the workflow fix was already merged previously, the PR description should not list it as a change being made here. Worth updating to avoid confusion about scope. ARCHITECTURE_DIAGRAM.mdThe rewrite looks accurate and readable. The new ASCII flow diagram is much cleaner than the old per-tool breakdown, and the One caveat I cannot verify without reading the source: does ChangelogReplacing the dev-note in Summary
The core changes are solid. The main thing worth addressing before merge is clarifying the workflow fix discrepancy in the PR description. 🤖 Reviewed with Claude Code |
Summary
fail_fast→fail-fastin.github/workflows/build.yml(was causing workflow parse error)build-all.ps1: simplified to match Makefile style; added$LASTEXITCODEguard aftergo buildand null-check for version extractionARCHITECTURE_DIAGRAM.md: full rewrite reflecting unifiedgomailtestbinary,internal/protocols/layout,internal/devtools/subcommand, and current test/CI structureCLAUDE.md: removed (was empty)ChangeLog/3.1.5.md: replaced dev note with user-facing changelog entriesChangeLog/3.1.6.md: created (was missing despite version bump)Test plan
.\build-all.ps1exits non-zero on build failure.\build-all.ps1prints clear error ifversion.gois missing/malformed🤖 Generated with Claude Code