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

schema: add clean to Makefile #774

Merged
merged 1 commit into from
Apr 21, 2017

Conversation

zhouhao3
Copy link

Signed-off-by: zhouhao [email protected]

@mrunalp
Copy link
Contributor

mrunalp commented Apr 18, 2017

LGTM

Approved with PullApprove

@@ -16,3 +16,6 @@ validate: validate.go
go get -d ./...
go build ./validate.go

clean:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also .PHONY.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think that is necessary. make clean can be executed correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that is necessary. `make clean1 can be executed correctly.

So can make validate, with or without the .PHONY declaration. .PHONY keeps the target working even if someone creates a clean file. And while that is unlikely, I don't see why we shouldn't protect ourselves from it.

Copy link
Author

Choose a reason for hiding this comment

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

If you do not add .PHONY, make validate can not be repeated, the following error occurs:
make: 'validate' is up to date.
so add the .PHONY.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you do not add .PHONY, make validate can not be repeated…

Fair. An example closer to your clean would be the the clean entry in the main Makefile.

@crosbymichael
Copy link
Member

crosbymichael commented Apr 21, 2017

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 4754b55 into opencontainers:master Apr 21, 2017
@zhouhao3 zhouhao3 deleted the makefile-clean branch April 24, 2017 03:09
@vbatts vbatts mentioned this pull request Jul 5, 2017
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.

4 participants