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

Add explicit library dependencies #703

Conversation

johanbrandhorst
Copy link
Contributor

Fixes #684.

Adds Gopkg.toml and Gopkg.lock with initial dependency versions. Includes workaround for Apache Thrift repo migration.

The versions used were chosen by dep init, and I welcome a discussion about exactly which direct dependency versions we want to use. The currently chosen versions allow all packages to build successfully so are a good starting point.

@johanbrandhorst
Copy link
Contributor Author

johanbrandhorst commented Apr 13, 2018

We'll probably want to discuss some procedure for updating dependencies as well, for example gRPC-Go has a (afaik) 3 monthly release cycle and we'll certainly want to stick to that. This may already be in place.

Copy link
Contributor

@semistrict semistrict left a comment

Choose a reason for hiding this comment

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

I propose we use major version ranges wherever possible in Gopkg.toml (version = "^1.X.Y"). Hopefully this works for everything except thrift.
@rakyll what do you think?


[[constraint]]
name = "google.golang.org/grpc"
version = "1.11.3"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should default to specifying major version ranges for everything, so: version = "^1.11.3" here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ah missed that, thanks :)

@semistrict
Copy link
Contributor

I think we need to be able to test this. What do you think of the following changes?:

  • add vendor/ to .gitignore
  • modify one of the Travis CI configurations to do "dep ensure" before building

That will make sure we are testing with the versions specified here but avoid vendoring the code.

@johanbrandhorst
Copy link
Contributor Author

@Ramonza That would remove the workaround @rakyll implemented for #675 (#677). As long as we're OK with relying on just dep for that, I don't see a problem.

@johanbrandhorst johanbrandhorst force-pushed the add-explicit-library-dependencies branch 3 times, most recently from c15f75d to c8f394d Compare April 16, 2018 07:44
Adds Gopkg.toml and Gopkg.lock with initial dependency
versions. Includes workaround for Apache Thrift repo migration.
Ignores the vendor folder and adds a travis test to ensure dependency
version updates don't break the build.

Fixes census-instrumentation#684.
@johanbrandhorst johanbrandhorst force-pushed the add-explicit-library-dependencies branch from c8f394d to cede0dd Compare April 16, 2018 07:58
@johanbrandhorst
Copy link
Contributor Author

Please re-review with the changes suggested.

@rakyll rakyll merged commit 36f8bed into census-instrumentation:master Apr 16, 2018
@johanbrandhorst johanbrandhorst deleted the add-explicit-library-dependencies branch April 16, 2018 21:46
@bweston92
Copy link
Contributor

When will this be in a release?

@rakyll
Copy link
Contributor

rakyll commented Apr 23, 2018

/cc @Ramonza, can we cut a new release?

@semistrict
Copy link
Contributor

Let's do a release after #728

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.

None yet

4 participants