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

tm-monitor Makefile updated #3790

Merged
merged 2 commits into from Jul 13, 2019

Conversation

sabau
Copy link
Contributor

@sabau sabau commented Jul 12, 2019

Makefile build-docker target updated to use go mod:

  • go111module on
  • switch from golang:alpine to golang:1.12 to have git
  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

- build docker needs go111module on
- switch to golang:1.12 to have git available

Signed-off-by: Karoly Albert Szabo <szabo.karoly.a@gmail.com>
@@ -36,7 +36,7 @@ dist: build-all

build-docker:
rm -f ./tm-monitor
docker run -it --rm -v "$(PWD)/../../:/go/src/github.com/tendermint/tendermint" -w "/go/src/github.com/tendermint/tendermint/tools/tm-monitor" -e "CGO_ENABLED=0" golang:alpine go build -ldflags "-s -w" -o tm-monitor
docker run -it --rm -v "$(PWD)/../../:/go/src/github.com/tendermint/tendermint" -w "/go/src/github.com/tendermint/tendermint/tools/tm-monitor" -e "GO111MODULE=on" -e "CGO_ENABLED=0" golang:1.12 go build -ldflags "-s -w" -o tm-monitor
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use golang latest image?

Copy link
Contributor Author

@sabau sabau Jul 12, 2019

Choose a reason for hiding this comment

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

Mainly precaution, if a new latest comes out and is slightly incompatible, we're safe at this hash/tag.

EDIT:
It's a thing I inherited from other languages, maybe go is less affected.

@codecov-io
Copy link

Codecov Report

Merging #3790 into master will decrease coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3790      +/-   ##
==========================================
- Coverage   64.34%   64.21%   -0.14%     
==========================================
  Files         214      214              
  Lines       18010    18010              
==========================================
- Hits        11589    11565      -24     
- Misses       5449     5466      +17     
- Partials      972      979       +7
Impacted Files Coverage Δ
privval/signer_validator_endpoint.go 75.55% <0%> (-10%) ⬇️
privval/socket_listeners.go 86.2% <0%> (-3.45%) ⬇️
blockchain/reactor.go 71.49% <0%> (-2.81%) ⬇️
blockchain/pool.go 80.26% <0%> (-1.98%) ⬇️
consensus/reactor.go 71.27% <0%> (-0.12%) ⬇️

@melekes melekes merged commit 6d96cf4 into tendermint:master Jul 13, 2019
@sabau sabau deleted the 1937-tm-monitor-build-docker branch July 14, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants