Skip to content

Build dmr.exe on Windows in build-dmr target#1006

Merged
ericcurtin merged 1 commit into
mainfrom
build-dmr-exe-windows
Jul 9, 2026
Merged

Build dmr.exe on Windows in build-dmr target#1006
ericcurtin merged 1 commit into
mainfrom
build-dmr-exe-windows

Conversation

@ericcurtin

@ericcurtin ericcurtin commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The build-dmr Makefile target always built a binary named "dmr",
    which is unusable on Windows because the OS requires a .exe suffix
    to execute it directly
  • Detect Windows via the built-in $(OS) variable and use a
    DMR_EXE variable for the output filename, so the target produces
    dmr.exe on Windows and dmr everywhere else. This avoids
    shelling out to go env GOEXE on every invocation, which adds
    overhead and can fail if go is not on the PATH
  • Reuse DMR_EXE in the clean target instead of hardcoding both
    dmr and dmr.exe so cleanup stays in sync with the build output
  • Update the README to match

Test plan

  • make validate-all (go mod tidy, lint, tests w/ race detection, shellcheck) passes
  • make build-dmr and make clean verified locally on macOS

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider reusing the DMR_EXE variable in the clean target instead of hardcoding both dmr and dmr.exe so the cleanup behavior automatically stays in sync with GOEXE-based naming.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider reusing the DMR_EXE variable in the clean target instead of hardcoding both dmr and dmr.exe so the cleanup behavior automatically stays in sync with GOEXE-based naming.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for building the dmr binary with a .exe extension on Windows by updating the Makefile and README. Feedback suggests replacing the shell call to 'go env GOEXE' with a check on the built-in '$(OS)' variable to avoid unnecessary overhead and potential build failures when Go is not in the PATH.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread Makefile Outdated
The build-dmr Makefile target always built a binary named "dmr",
which is unusable on Windows because the OS requires a .exe suffix
to execute it directly. Detect Windows via the built-in $(OS)
variable and use a DMR_EXE variable for the output filename, so the
target produces dmr.exe on Windows and dmr everywhere else. This
avoids shelling out to "go env GOEXE" on every invocation, which adds
overhead and can fail if go is not on the PATH. Reuse DMR_EXE in the
clean target instead of hardcoding both dmr and dmr.exe so cleanup
stays in sync with the build output. Update the README to match.
@ericcurtin ericcurtin force-pushed the build-dmr-exe-windows branch from 96e2eea to 2020eb5 Compare July 9, 2026 12:11
@ericcurtin ericcurtin merged commit d1449b2 into main Jul 9, 2026
14 checks passed
@ericcurtin ericcurtin deleted the build-dmr-exe-windows branch July 9, 2026 13:52
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