Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3613 +/- ##
===========================================
+ Coverage 63.33% 63.36% +0.03%
===========================================
Files 217 217
Lines 18169 18127 -42
===========================================
- Hits 11507 11487 -20
+ Misses 5701 5680 -21
+ Partials 961 960 -1
|
| - save_cache: | ||
| key: v1-release-deps-{{ .Branch }}-{{ .Revision }} | ||
| paths: | ||
| - "vendor" |
There was a problem hiding this comment.
Please, do not delete the parts that enable a cache (but make it use modules too)!
There is still a
- restore_cache:
keys:
- v1-release-deps-{{ .Branch }}-{{ .Revision }}left below. If there are any good reasons to not use a cache, then we'd need to delete those lines too.
liamsi
left a comment
There was a problem hiding this comment.
Cool! Thanks @jackzampolin!
While the save_cache part in the circle config was deleted, there still is a restore_cache part which does not make sense. Ideally, we continue using circleci's cache but with modules.
Some of the added GO111MODULE=on could be removed, if we do not store the code under $GOPATH/src. This might simplify the circle config, too. The latter can be done in a followup PR tho.
|
Any way we could do the rest in a follow up PR. I'm not super firmiliar with where |
|
We should remove both
You can have a look here: https://github.com/tendermint/go-amino/blob/fc5dc13311ae690b06c40efb18379976f962c6ce/.circleci/config.yml#L9-L22 I can take this over in a follow-up PR, too. |
This reverts commit 45117bd Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
|
As commented above only the cache with the key |
of vendored deps; also: - bump version for dependency cache - bump version on pkg-cache (includes modules directory) Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
|
See: #3623 Independent from that, these need to be fixed too (before merging this):
Maybe in a follow-up PR:
|
melekes
left a comment
There was a problem hiding this comment.
Cool ⛷ 2 questions:
- Should we also remove Gopkg.toml and Gopkg.lock files?
- Can we have a changelog pending entry?
Also see my comments about structuring go.mod, etc.
| id=$(basename "$pkg") | ||
|
|
||
| go test -v -timeout 5m -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic "$pkg" | tee "/tmp/logs/$id-$RANDOM.log" | ||
| GO111MODULE=on go test -v -timeout 5m -mod=readonly -race -coverprofile=/tmp/workspace/profiles/$id.out -covermode=atomic "$pkg" | tee "/tmp/logs/$id-$RANDOM.log" |
There was a problem hiding this comment.
Won't it be better to set the env var in top-level defaults?
defaults: &defaults
environment:
GO111MODULE: on
CONTRIBUTING.md
Outdated
| Since some dependencies are not under our control, a third party may break our | ||
| build, in which case we can fall back on `dep ensure` (or `make | ||
| get_vendor_deps`). Even for dependencies under our control, dep helps us to | ||
| build, in which case we can fall back on `go mod tidy`. Even for dependencies under our control, dep helps us to |
There was a problem hiding this comment.
Even for dependencies under our control, dep helps us to
=>
Even for dependencies under our control, go mod helps us to
| go 1.12 | ||
|
|
||
| require ( | ||
| github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973 |
There was a problem hiding this comment.
can we structure this like Gopkg.toml?
# Allow only patch releases for serialization libraries
...
# Allow only minor releases for other libraries
## XXX
...
## YYY
...
## Test libraries
...
There was a problem hiding this comment.
Related note to myself: double check that all dependencies from the latest develop survived the mode to go modules (e.g. can't find "github.com/etcd-io/bbolt" which was introduced after this PR was opened)
There was a problem hiding this comment.
@melekes looks like part of the tooling will rewrite the go.mod file to be alphabetically sorted...
As this PR currently removes |
|
I like this. We recently migrated to go mod as well, but had to drop support for everything less than 1.11.4 (early 1.11 had some issues with go mod). If you are going to break golang version support, breaking dep support at the same time is fine in my opinion. Maybe good to have a stable release (0.31.5 is it?) that supports the 1.10 and dep, then just make a breaking change to build but not many features to allow an easy upgrade path for project (just the build system and not the apis) |
We are already expecting go1.11.4 or higher for a while now: https://github.com/tendermint/tendermint#minimum-requirements |
- remove Gopkg.(toml | lock) - update contributing guidlines - set global default in circleci (GO111MODULE=on) Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
`go: unknown environment setting GO111MODULE=true` although the var was `GO111MODULE: on` Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
|
On another note. Can this file be deleted? I points to an archived repo... cc @melekes tendermint/DOCKER/Dockerfile.abci Lines 6 to 23 in a076b48 |
Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
Signed-off-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
There was a problem hiding this comment.
I think we can merge this at this is now. We should probably open a follow-up issue to deal with the outdated and broken abci docker image (see: #3613 (comment))
We should play around with this on develop before we add this to the next breaking release though.
| github.com/syndtr/goleveldb v0.0.0-20181012014443-6b91fda63f2e | ||
| github.com/tendermint/go-amino v0.14.1 | ||
| golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25 | ||
| golang.org/x/net v0.0.0-20180906233101-161cd47e91fd |
There was a problem hiding this comment.
looks like this got updated!
| github.com/tendermint/go-amino v0.14.1 | ||
| golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25 | ||
| golang.org/x/net v0.0.0-20180906233101-161cd47e91fd | ||
| golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a |
There was a problem hiding this comment.
looks like this got updated!
| INCLUDE = -I=. -I=${GOPATH}/src -I=${GOPATH}/src/github.com/gogo/protobuf/protobuf | ||
| BUILD_TAGS?='tendermint' | ||
| BUILD_FLAGS = -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD`" | ||
| BUILD_FLAGS = -mod=readonly -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD`" |
|
https://github.com/btcsuite/btcutil Looks like the most important change is btcsuite/btcutil@0ecd90b lowering |
changes http2 and html packages |
|
This PR switches dependency management to using
go mod.