Build release binary with Go#53
Merged
Merged
Conversation
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR replaces the direct ChangesGo Binary Build CLI with Platform Targeting and Version Injection
Sequence Diagram(s)sequenceDiagram
participant Script as scripts/build.ts
participant Parser as parseBuildArgs
participant Reader as parsePackageVersion
participant Builder as buildZeroBinary
participant GoBuild as "go build"
Script->>Parser: parse CLI args / env
Script->>Reader: read package.json -> version
Script->>Builder: invoke buildZeroBinary(goos, goarch, output, version)
Builder->>GoBuild: spawn go build with -ldflags (version)
GoBuild-->>Builder: stdout/stderr, exit status
Builder-->>Script: success or throw
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/build-scripts.test.ts (1)
65-83: ⚡ Quick winAdd a regression test for short-flag value rejection.
Please add a case like
parseBuildArgs(['-o', '-h'])(and/or['--goarch', '-h']) expecting a throw, so the parser behavior is locked for this CLI edge path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/build-scripts.test.ts` around lines 65 - 83, Add a regression test in tests/build-scripts.test.ts that verifies parseBuildArgs rejects a short-flag value that looks like a flag (e.g. call parseBuildArgs(['-o', '-h']) and/or parseBuildArgs(['--goarch', '-h'])) by expecting it to throw; locate the existing Go binary tests using the describe block 'Go binary build script' and add an it case asserting that parseBuildArgs throws for those inputs to lock the CLI edge-case behavior for the parseBuildArgs function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build.ts`:
- Around line 185-191: The requireValue function currently only treats values
starting with '--' as missing, allowing short flags like '-h' to be mistaken for
values; update requireValue (args, index, flag) to reject any value that starts
with '-' (single or double) — e.g., check value is truthy and does not match
/^-/ or use value.startsWith('-') — and throw the same `${flag} requires a
value` error when a short or long flag is encountered so callers like
requireValue(...) will not accept '-h' or '-o' as valid values.
---
Nitpick comments:
In `@tests/build-scripts.test.ts`:
- Around line 65-83: Add a regression test in tests/build-scripts.test.ts that
verifies parseBuildArgs rejects a short-flag value that looks like a flag (e.g.
call parseBuildArgs(['-o', '-h']) and/or parseBuildArgs(['--goarch', '-h'])) by
expecting it to throw; locate the existing Go binary tests using the describe
block 'Go binary build script' and add an it case asserting that parseBuildArgs
throws for those inputs to lock the CLI edge-case behavior for the
parseBuildArgs function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ba173577-3c7c-4332-b274-7a0b895a8fc9
📒 Files selected for processing (14)
.github/workflows/ci.yml.github/workflows/release-artifacts.ymlREADME.mddocs/NPM_WRAPPER_SMOKE.mddocs/PERFORMANCE.mdinternal/cli/app.gointernal/cli/app_test.gopackage.jsonscripts/artifact.tsscripts/build.tsscripts/package-release.tsscripts/smoke-build.tstests/build-scripts.test.tstests/foundation-tools.test.ts
anandh8x
force-pushed
the
feat/m1-binary-build
branch
from
June 4, 2026 11:11
7a79faa to
b55d408
Compare
anandh8x
force-pushed
the
feat/m1-binary-build
branch
from
June 4, 2026 11:13
b55d408 to
9244258
Compare
Collaborator
|
Blockers Non-Blocking
Looks Good
|
Vasanthdev2004
approved these changes
Jun 4, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Approved current head 9244258 after reviewing the Go release-binary build path, verifying the parser edge fix, checking CI Go validation, and validating both PR branch and latest-main test-merge TS/Bun paths.
gnanam1990
approved these changes
Jun 4, 2026
gnanam1990
left a comment
gnanam1990
left a comment
Collaborator
There was a problem hiding this comment.
Approved current head 9244258ea0b93e8700ced1387939e20ef337a75c after rechecking the build-script changes, the previously requested short-flag parser fix, and the release binary flow.
Notes:
- The earlier CodeRabbit
requireValuefinding is fixed in current code: values starting with-are rejected, and the regression test covers['-o', '-h']and['--goarch', '-h']. - CodeRabbit's latest pass says no actionable comments were generated.
- I did not find remaining blockers in the changed release/build path.
Validation run:
bun install --frozen-lockfilepassedbun test tests/build-scripts.test.ts tests/foundation-tools.test.ts --timeout 15000passedbun test tests/install-scripts.test.ts tests/release-checksums.test.ts --timeout 15000passedbun run typecheckpassedgo test ./...passedbun run buildpassed and produced the Go binary with version0.1.0bun run smoke:buildpassedbun run smoke:gopassedbun test ./tests --timeout 15000passed, 285 pass / 0 failgit diff --check origin/main...HEADpassed
Collaborator
|
@coderabbitai review |
✅ Action performedReview finished.
|
Vasanthdev2004
dismissed
coderabbitai[bot]’s stale review
June 4, 2026 13:54
Fixed in 9244258; latest CodeRabbit status is green and maintainers approved current head.
This was referenced Jun 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Full suite note
Summary by CodeRabbit
Build System
Documentation
Chores