Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix makefile help #371

Merged
merged 2 commits into from Apr 9, 2024
Merged

Fix makefile help #371

merged 2 commits into from Apr 9, 2024

Conversation

wszaranski
Copy link
Contributor

Add Makefile help comments

Commit that originally introduced make help target also had special comments (starting with ###) for help target to parse.
Makefile that was finally merged after some changes lacked this comments. Without this comments when executed help prints empty output.


Move .PHONY next to each target

It's clearer to keep target and it's .PHONY together. When reading Makefile it's easier to see if target is phony or not (without jumping back to first line and reading all of .PHONY dependencies). It's also harder to forget remove/rename .PHONY dependency when target is changed when they are together.

There is no official standard but make documentation in .DEFAULT_GOAL use multiple .PHONY:

.PHONY: foo
foo: ; @echo $@

$(warning default goal is $(.DEFAULT_GOAL))

# Reset the default goal.
.DEFAULT_GOAL :=

.PHONY: bar
bar: ; @echo $@

Commit [1] that originally introduced make help target also had special
comments (starting with ###) for help target to parse.
Makefile that was finally merged after some changes lacked this
comments. Without this comments when executed help prints empty output.

PR: alicebob#371
[1]: alicebob@72133e9

Signed-off-by: Wojciech Szarański <wojciech.szaranski@gmail.com>
It's clearer to keep target and it's .PHONY together. When reading
Makefile it's easier to see if target is phony or not (without jumping
back to first line and reading all of .PHONY dependencies). It's also
harder to forget remove/rename .PHONY dependency when target is changed
when they are together.

There is no official standard but make documentation[1] in .DEFAULT_GOAL
use multiple .PHONY:
> .PHONY: foo
> foo: ; @echo $@
>
> $(warning default goal is $(.DEFAULT_GOAL))
>
> # Reset the default goal.
> .DEFAULT_GOAL :=
>
> .PHONY: bar
> bar: ; @echo $@

PR: alicebob#371
[1]: https://www.gnu.org/software/make/manual/html_node/Special-Variables.html

Signed-off-by: Wojciech Szarański <wojciech.szaranski@gmail.com>
@alicebob
Copy link
Owner

alicebob commented Apr 9, 2024

Oh, the ### ones were special, learned something new today. Sorry for messing up and thanks for fixing!

@alicebob alicebob merged commit 5275a76 into alicebob:master Apr 9, 2024
4 checks passed
alicebob pushed a commit that referenced this pull request Apr 9, 2024
Commit [1] that originally introduced make help target also had special
comments (starting with ###) for help target to parse.
Makefile that was finally merged after some changes lacked this
comments. Without this comments when executed help prints empty output.

PR: #371
[1]: 72133e9

Signed-off-by: Wojciech Szarański <wojciech.szaranski@gmail.com>
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.

None yet

2 participants