Skip to content

Makefile: declare every target phony, and stop hiding integ-arm64 from help - #38

Closed
calvinchengx wants to merge 2 commits into
fpt:mainfrom
calvinchengx:makefile-hygiene
Closed

calvinchengx wants to merge 2 commits into
fpt:mainfrom
calvinchengx:makefile-hygiene

Conversation

@calvinchengx

Copy link
Copy Markdown
Contributor

Two small Makefile fixes, independent of each other and of #37.

.PHONY listed only integ

Every other target was a real file target. A file or directory named test, run, fmt, lint or help in the repository root silently turns the matching target into a no-op — and it exits 0, so CI would go green without running anything:

$ make test
tests ran
$ touch test
$ make test
make: `test' is up to date.
$ echo $?
0

test is a plausible name for someone to create. Now all seven targets are declared.

make help hid integ-arm64

The pattern was ^[a-zA-Z_-]+: — no digit class — so integ-arm64 carried a ## comment that make help never printed. Adding 0-9 fixes it, and it stays correct for any future target with a version or architecture in its name.

One comment, no behaviour change

I also added a header saying what the Makefile is: convenience assuming GNU make and a POSIX shell, rather than the build. The build and tests are the Go toolchain alone, which is what lets the Windows CI job run go test ./... directly. That felt worth stating where someone on Windows would look first, but it's a comment — drop that commit if you'd rather not carry it.

Verified with make help and make test.

Without this, a file or directory named test, run, fmt, lint or help in
the repository root silently turns the matching target into a no-op.
The pattern had no digit class, so integ-arm64 was documented with a ##
comment that make help never printed.
@calvinchengx

Copy link
Copy Markdown
Contributor Author

Already on main, in fuller form: the header comment, the complete .PHONY list and the a-zA-Z0-9_- fix in help (line 52) are all there. Closing as landed, not as rejected.

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.

1 participant