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

Vendor dependencies #150

Closed
wants to merge 4 commits into from
Closed

Vendor dependencies #150

wants to merge 4 commits into from

Conversation

petemiron
Copy link
Contributor

Resolves #149

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.162% when pulling 3f6468c on vendor_dependencies into d99acc1 on master.

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

Don't have to make the changes, but may want to use other approach for misspell to try be consistent.
I am wondering if this is going to cause issues in the server since server vendors go-nats-streaming and others. Some protobuf thing will then be vendored twice in the server?

.travis.yml Outdated
- EXCLUDE_VENDOR=$(go list ./... | grep -v "/vendor/")
- $(exit $(go fmt $EXCLUDE_VENDOR | wc -l))
- go vet $EXCLUDE_VENDOR
- find . -type f -name '*' | grep -v vendor/ | xargs misspell -error
Copy link
Member

Choose a reason for hiding this comment

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

Or, to be inline with other repos and what we do about, you could do:

$(exit $(misspell -locale US . | grep -v "vendor/" | wc -l))

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.162% when pulling 09b431c on vendor_dependencies into d99acc1 on master.

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

I still find it hard to swallow to vendor gnatsd because it is used in tests. Tests are not vendor'ed but because you vendor go-nats and test.go imports gnatsd, we end-up importing it. What if test.go was removed and functions defined there were moved to some _test.go file? The danger is that as of now these are exported and since there were in test.go they are part of the library. We may break things by moving them. Care to experiment?

@petemiron petemiron closed this Aug 18, 2017
@petemiron petemiron deleted the vendor_dependencies branch August 18, 2017 21:00
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