-
Notifications
You must be signed in to change notification settings - Fork 19
chore(ci): Add makefile to simplify commands. #181
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
Conversation
mikecdavis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor clean-up needed, but other than that this is a big improvement to the overall dev experience :)
| # The name of the executable (default is current directory name) | ||
| TARGET := $(shell basename "$(PWD)") | ||
| VERSION ?= $(shell git describe --tags) | ||
| .DEFAULT_GOAL := help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently missing a help target but can define as:
help: ## help
@awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST)
Makefile
Outdated
| @@ -0,0 +1,31 @@ | |||
| # The name of the executable (default is current directory name) | |||
| TARGET := $(shell basename "$(PWD)") | |||
| VERSION ?= $(shell git describe --tags) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used
Makefile
Outdated
| GO111MODULE=$(GO111MODULE) $(GOTEST) ./pkg/... | ||
|
|
||
| test-ci: ## run unit tests in CI mode | ||
| GO111MODULE=$(GO111MODULE) $(GOTEST) -v -race ./pkg/... -coverprofile=profile.cov No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-v is not needed and can make for noisy logs. If there's a failure then adequate logs are still exposed. Consider naming this target cover as it's not doing anything particularly really CI related that you wouldn't want to run locally. It was also confusing in the .travis.yml file that some tasks used test and just one was test-ci.
.travis.yml
Outdated
| - curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.19.0 | ||
| script: | ||
| - $GOPATH/bin/golangci-lint run --out-format=tab --tests=false pkg/... | ||
| - make -e install lint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: -e isn't needed given your current Makefile
Makefile
Outdated
| GOTEST=$(GOCMD) test | ||
| GOGET=$(GOCMD) get | ||
| GOLINT=golangci-lint | ||
| BINARY_UNIX=$(TARGET)_unix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used.
| GOBIN=bin | ||
| GOPATH=$(shell $(GOCMD) env GOPATH) | ||
| GOBUILD=$(GOCMD) build | ||
| GOCLEAN=$(GOCMD) clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to enable a clean target.
mikecdavis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but can also get rid of references to GOBIN since you're not using a build target, etc.
Summary