Code review comment for lp:~allenap/juju-core/makefile-stuff
Revision history for this message
| Gavin Panella (allenap) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Thanks for the reviews, Roger & John.
https:/
File Makefile (right):
https:/
Makefile:10: PROJECT_DIR := $(shell go list -e -f '{{.Dir}}' $(PACKAGE))
On 2013/08/28 06:23:17, jameinel wrote:
> You are using "PROJECT_DIR := .... $(PACKAGE)"
> but I don't see where PACKAGE is getting set. Did you mean to use
PROJECT there?
Oh, embarrassing. `go list` does the right thing when in the project
directory on GOPATH so I didn't spot this. Fixed, thanks for spotting
that.
https:/
Makefile:44: $(error Cannot build package outside of GOPATH)
On 2013/08/28 06:23:17, jameinel wrote:
> 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.
I think it's good to keep it. For example, if someone runs `make build`
from a tree that's not on GOPATH it will build a different tree
(assuming one exists) that is on GOPATH, and it'll get confusing. I'll
change the message though, to something like "Cannot build; $(CURDIR) is
not found in GOPATH". I think that better explains what's happening.
https:/
Makefile:64: gofmt -w -l -s .
On 2013/08/28 06:23:17, jameinel wrote:
> 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 .
Okay, I'll do that.
> 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*.
Instead of using `make -f ../../Makefile format` you can use `make -C
../.. format` which will DTRT for you, and is shorter.
https:/