Code review comment for lp:~allenap/juju-core/makefile-stuff
Revision history for this message
| John A Meinel (jameinel) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Some comments, but overall LGTM
https:/
File Makefile (right):
https:/
Makefile:10: PROJECT_DIR := $(shell go list -e -f '{{.Dir}}' $(PACKAGE))
You are using "PROJECT_DIR := .... $(PACKAGE)"
but I don't see where PACKAGE is getting set. Did you mean to use
PROJECT there?
eg:
PROJECT_DIR := $(shell go list -e -f '{{.Dir}}' $(PROJECT))
https:/
Makefile:44: $(error Cannot build package outside of GOPATH)
This is really more of "you must be at $PROJECT_DIR to run build".
Which isn't strictly true from my experience.
You can run "go build launchpad.
everything under there just fine. (I often use syntax similar to that
when I'm in a subdir and want to build something in a sibling dir.)
But given that Makefile is at the root, I don't think people will really
do:
make -f ../../../Makefile
anyway, so I don't think it is a big deal. So if it helps you to have
the check for CWD you can leave it in.
https:/
Makefile:64: gofmt -w -l -s .
Given that "time make format" and "time make simplify" doen't seem to
differ, we probably just want to always default to simplify.
So:
format: simplify
simplify:
gofmt -w -l -s .
Though really *I* prefer
go fmt $(PROJECT)/...
Because that makes sure I get the files in another directory. (If I'm in
the subdir cmd/juju I'm probably also hacking on code in environs/tools
or whatever)
Then again, Makefile isn't available easily in subdirs, so it doesn't
matter *for me*.
https:/