Add a make build target and repoint README's last raw go build instruction
#19
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
README.md's "Building and running" section still tells the reader to run:This is the last raw-toolchain instruction left in the docs. #3 fixed the test
path (
go test ./game/→make test) but deliberately left this one alone,correctly: there is no
build:target to point it at, and adding one inside adocs-only commit would have broken the docs-only review exception that PR
relied on. So the target has to come first.
The org standard expects the Makefile to be the interface to the toolchain —
make buildis one of the standard targets, and this repo's Makefile isotherwise a complete (if minimal) set:
fmt,fmt-check,lint,test,check.Definition of done
build:target in the Makefile that builds the binary, following thefile's existing conventions (a short comment above it like its neighbors,
listed in
.PHONY).buildis wired into neitherchecknortest.make checkmust stayexactly
fmt-check lint test, and must continue not to modify any file inthe repo — verify by running
make checkand confirminggit statusisclean afterwards.
.gitignoreif thebinary would otherwise land in the working tree. A stray
roguebinarymust not be committable.
README.md's "Building and running" block usesmake build, and themake-target list in the "Code layout" section mentions it.
goinvocation remains anywhere inREADME.md— grep and confirm.make checkfully green.TODO.mdupdated in the same commit — add a Completed Steps entry anddo not rotate "Next Step" (precedent ratified on PR #9: out-of-band
issue work leaves Next Step alone).
(closes #N).Implementation requirements
maketargets only for verification — never rungo build/go testdirectly, including to check your own target works. Invoke
make build.script/entrypoints — this repo isexplicitly exempt.
.golangci.yml.c-masterandmodern-roguebranches alone.make fmtfor the markdown and include the result in the commit.Explicitly out of scope
The org Go styleguide also wants the git commit hash embedded into the binary
and surfaced at startup (the
-X main.Versionpattern). That is a real gaphere, but it is a separate concern with its own design questions — do not
fold it in. If you think it is worth doing, say so on this issue and I will
file it separately.
Priority
Low. Cosmetic/consistency, no defect behind it.
Done in
60442ceonnext, in #46.make buildbuilds tobuild/rogue.build/is the one place generated artifacts go and.gitignorecovers the whole directory, so a stray binary is not committable.buildis in neitherchecknortest;checkis still exactlyfmt-check lint test, and its recipe now carries a comment saying only non-writing targets belong in that list.README.md: the "Building and running" block usesmake build/./build/rogue, the four run examples and the wizard-mode example follow, and the make-target list in "Code layout" namesbuild.grep -nE '\bgo (build|test|run|vet|tool|install|get|mod|fmt|generate)\b' README.mdreturns nothing.TODO.mdhas a Completed Steps entry in the same commit; "Next Step" is untouched.Verification,
maketargets only:make buildprintedgo build -o build/rogue ./cmd/rogueand produced the binary;git status --porcelainempty with it present.GOFLAGS=-count=1 make checkgreen in 22.5s. The lint layer reportedDONE 13.2swith0 issues., notCACHED, so it executed; the test lines carry real durations (cmd/rogue 1.075s,game 4.049s) with no(cached)marker.git status --porcelainempty afterwards.One thing outside what the issue asked for:
.dockerignorenow excludes/build/. The new target puts a 5.4MB binary inside the repo, which was otherwise shipped into the lint build context — the context transferred 6.25MB with the binary present and 72.32kB after the exclusion. Nothing excluded is a Go source,go.mod/go.sumor.golangci.yml.On the out-of-scope item: embedding the commit hash via
-X main.Versionis worth doing, and it is bigger than a linker flag — there is no version surface in the binary at all right now, so it needs amain.Versionvar, a decision about where it is displayed (startup line, a-vflag, or the score screen), and a fallback for builds from a dirty or absent working tree. Worth its own issue.