Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

no need to go get anything. since we use vendoring #385

Merged
merged 3 commits into from
Nov 15, 2016
Merged

Conversation

Dieterbe
Copy link
Contributor

No description provided.

@Dieterbe
Copy link
Contributor Author

shaves off about 25s from circleci builds

@@ -1,14 +1,12 @@
.PHONY: test bin docker
default:
$(MAKE) all
test:
Copy link
Member

Choose a reason for hiding this comment

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

i would like to have a test target. So if the scripts/test.sh doesnt exist, lets create it and have it run go vet and go test -race

@Dieterbe
Copy link
Contributor Author

how is this? I think go vet tends be sometimes a tad too strict.
for example it always complains about all our instantiations of schema.Point:

                    Datapoints: []schema.Point{
                        {123, 60},
                        {10000, 120},
                        {0, 180},
                        {1, 240},
                    },

it wants us to specify explicitly the keys in those fields, which makes everything soo verbose 👎

@woodsaj
Copy link
Member

woodsaj commented Nov 14, 2016

go vet is not too strict. It enforces best practices.

composite literals whose type is defined in a separate package should use the keyed notation.

But fixing the code is outside the scope of this change so i have created a separate issue.
#388

@@ -2,13 +2,13 @@
default:
$(MAKE) all
test:
bash -c "./scripts/test.sh $(TEST)"
CGO_ENABLED=1 go test -v -race $(go list ./... | grep -v /vendor/)
Copy link
Member

Choose a reason for hiding this comment

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

why are you enabling CGO, shouldn't it be disabled as is the case for building

Copy link
Contributor Author

@Dieterbe Dieterbe Nov 14, 2016

Choose a reason for hiding this comment

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

$ go test -v -race $(go list ./... | grep -v /vendor/)                                                                                                                ⏎
go test: -race requires cgo; enable cgo by setting CGO_ENABLED=1

@Dieterbe Dieterbe merged commit 01ae5e3 into master Nov 15, 2016
@Dieterbe Dieterbe deleted the quicker-ci branch December 15, 2017 22:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants