Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 3, 2026

Code review identified critical build configuration errors that would prevent Windows 32-bit binaries from building and cause incorrect exit codes when binaries are missing.

Changes

  • Windows 32-bit build: Use i686-w64-mingw32-gcc instead of x86_64-w64-mingw32-gcc for 386 architecture
  • Error handling: Replace || with ?? operator in wrapper script to handle null status correctly
  • CI dependencies: Add gcc-mingw-w64-i686 package to both release and test workflows
  • Darwin builds: Disable CGO for cross-compilation (codebase doesn't use C dependencies)

Before

process.exit(error.status || 1);  // exits with 0 if status is null
CC=x86_64-w64-mingw32-gcc  # wrong compiler for 32-bit

After

process.exit(error.status ?? 1);  // correctly exits with 1 if status is null
CC=i686-w64-mingw32-gcc  # correct 32-bit compiler

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…error handling, and CGO settings

Co-authored-by: leggetter <328367+leggetter@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 3, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • esm.ubuntu.com
    • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix pnpm v10+ compatibility with GoReleaser fix: correct Windows 32-bit compiler and error handling in npm build Feb 3, 2026
Copilot AI requested a review from leggetter February 3, 2026 18:53
@leggetter
Copy link
Collaborator

@copilot does 7002310 address your feedback?

Copy link
Contributor Author

Copilot AI commented Feb 3, 2026

@copilot does 7002310 address your feedback?

Yes, commit 7002310 addresses my feedback and takes an even better approach. Instead of fixing the Windows 32-bit compiler (which I suggested), it disables CGO entirely for all platforms since the codebase has no C dependencies. This is cleaner - it eliminates the need for mingw cross-compilers completely and simplifies the build process.

The commit addresses all my review points:

  • ✅ Error handling fix (error.status ?? 1)
  • ✅ Darwin CGO disabled for cross-compilation
  • ✅ Windows builds simplified (no compiler dependencies needed)
  • ✅ Enhanced test coverage for all 6 platforms

The pure Go approach (CGO_ENABLED=0) is the right solution here.

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.

2 participants