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

CI: Make CI not update the lock file #2152

Merged
merged 3 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: binaries
command: |
Expand All @@ -51,7 +51,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: Get metalinter
command: |
Expand All @@ -74,7 +74,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: Test cli
command: |
Expand All @@ -93,7 +93,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: Test individual module simulations
command: |
Expand All @@ -111,7 +111,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: Test individual module simulations
command: |
Expand All @@ -129,7 +129,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: Test full Gaia simulation
command: |
Expand All @@ -147,7 +147,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run: mkdir -p /tmp/logs
- run:
name: Run tests
Expand Down Expand Up @@ -176,7 +176,7 @@ jobs:
name: dependencies
command: |
export PATH="$GOBIN:$PATH"
make get_vendor_deps
make get_vendor_deps_ci
- run:
name: gather
command: |
Expand Down Expand Up @@ -207,7 +207,7 @@ jobs:
command: |
set -x
make get_tools
make get_vendor_deps
make get_vendor_deps_ci
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember we had something like that in Tendermint, but switched to make get_vendor_deps. Does this mean we should do the same change in Tendermint?

Copy link
Contributor Author

@ValarDragon ValarDragon Aug 27, 2018

Choose a reason for hiding this comment

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

Probably. We made that change before we ensured the lock file is correct, but now that we do, we can go back to how we used to do it Tendermint!

make build-linux
make localnet-start
./scripts/localnet-blocks-test.sh 40 5 10 localhost
Expand Down
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ get_vendor_deps:
@rm -rf .vendor-new
@dep ensure -v

get_vendor_deps_ci:
Copy link
Contributor

@alexanderbez alexanderbez Aug 26, 2018

Choose a reason for hiding this comment

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

Almost an identical target to get_vendor_deps right? Why not just add a build env var/flag (i.e. @dep ensure -v $(VENDOR)) to the existing target (where the default value is false)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. I think makefiles tend to be super hard to read, so I prefer not using flags for the simple things in the main commands.

I don't feel strongly about this, so if you'd prefer flags id be happy to change it

Copy link
Contributor

Choose a reason for hiding this comment

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

Nor do I -- you can keep as is.

@echo "--> Running dep ensure"
@rm -rf .vendor-new
@dep ensure -v -vendor-only

draw_deps:
@# requires brew install graphviz or apt-get install graphviz
go get github.com/RobotsAndPencils/goviz
Expand Down