Add Go modules support but keep dep support #255
Conversation
liamsi
force-pushed
the
switch_to_gomodules
branch
from
6efb99f
to
3c50a13
Compare
- remove dep artifacts - remove traces of dep from make file: go build will handle those for us - update circelci config to go1.12
liamsi
force-pushed
the
switch_to_gomodules
branch
from
3c50a13
to
47d58e5
Compare
liamsi
force-pushed
the
switch_to_gomodules
branch
from
41350f1
to
9e26b55
Compare
- do we need to add all those deps to go.sum, too?
liamsi
force-pushed
the
switch_to_gomodules
branch
from
9e26b55
to
c16f67e
Compare
- otherwise tests fail (bug in testify) -> deal with this in a separate PR
|
Interestingly, some combination of updated dependencies reveals a failing test: |
|
Meh, I've opened #256 |
- some go vet lints (redundant type)
updated versions in modules)
liamsi
changed the title
addressed in followup PR)
Makefile
Outdated
|
|
||
| # To avoid unintended conflicts with file names, always add to .PHONY | ||
| # unless there is a reason not to. | ||
| # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html | ||
| .PHONY: build install check_tools get_tools update_tools get_vendor_deps test fmt metalinter metalinter_all | ||
| .PHONY: build install check_tools get_tools update_tools test |
There was a problem hiding this comment.
add lint? why remove fmt?
I am against removing get_vendor_deps. We'll have to change all documentation + developers are forced to learn a new way! Why not just call go modules install or whatever instead of dep ensure?
There was a problem hiding this comment.
Hmm, the magic behind go modules is also that you do not need an extra step. go build or go test will fetch all the deps from mod.go / mod.sum for you (for go >= 1.11)... There is go mod vendor but it isn't necessary. Also:
By default, go commands like go build ignore the vendor directory when in module mode.
https://github.com/golang/go/wiki/Modules#how-do-i-use-vendoring-with-modules-is-vendoring-going-away
I only left Gopkg.toml for projects that do not yet use modules (but dep) and import go-amino (for them nothing changes; but they do not need the make file either).
Maybe, we should update the documentation though?
There was a problem hiding this comment.
Oh.. I don't have to do anything, cool! Then we can drop get_vendor_deps, thanks 🙇
There was a problem hiding this comment.
add lint? why remove fmt?
Added lint to .PHONY. I thought it might be better to run fmt through lint / golangci-lint. Added fmt to phony.
Makefile
Outdated
|
|
||
| ######################################## | ||
| ### Testing | ||
|
|
||
| test: | ||
| go test $(shell go list ./... | grep -v vendor) | ||
| go test -v $(shell go list ./... | grep -v vendor) |
There was a problem hiding this comment.
I wasn't seeing what exactly was going on with the failing tests. Can revert though.
|
Thanks @melekes! |
cc @jackzampolin