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

makefile: clean test #322

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -35,7 +35,7 @@ clean:
$(RUNTIME_TOOLS_LINK):
ln -sf $(CURDIR) $(RUNTIME_TOOLS_LINK)

.PHONY: test .gofmt .govet .golint
.PHONY: test

Choose a reason for hiding this comment

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

Well, I think this is not a bug or wrong content.
Putting them into .PHONY is a way to defend in case of there is .gofmt, .govet and so son which will break make process.
As there is not any one in project directory, removing them or not is got a big deal.
But noting fix in comment is not suitable, cleanup will be better.

Copy link
Author

@zhouhao3 zhouhao3 Feb 10, 2017

Choose a reason for hiding this comment

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

Both cases can be implemented, and I did not find an accurate description of which is wrong. Just a personal feeling that would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Putting them into .PHONY is a way to defend in case of there is .gofmt, .govet and so son which will break make process.

And it's also a way of saying “these target names are just for convenience. We are not attempting to create files called .gofmt, etc.”. I'd rather leave them .PHONY so folks don't get confused (“Why is this recipe not creating a .gofmt file?”).


test: .gofmt .govet .golint

Expand Down