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

use standard context package instead of x/net/context #174

Merged
merged 3 commits into from
Oct 22, 2018

Conversation

achille-roussel
Copy link
Contributor

Following up on our conversation in #149

This PR modifies the source files to use the standard context package instead of x/net/context. This means that this repository will not be compatible with Go 1.6 and earlier anymore. The README does not mention the minimum Go version currently supported so I'm not sure if this is going to be a deal-breaker or not.

If we need support for Go 1.6 we can probably figure out a way to support either packages with a compile-time check, but it could introduce lots of complexity in the code.

I was concerned that x/net/http2 may have used x/net/context directly but it seems they have moved to the standard context package already, there's only one leftover in a test:

github.com/golang/net/http2 $ grep -r 'x/net/context' .
./transport_test.go:	"golang.org/x/net/context"

There are two places where x/net/context is still referenced after this change:

lightstep-tracer-go $ grep -r x/net/context .
./collectorpb/collectorpbfakes/fake_collector_service_client.go:	"golang.org/x/net/context"
./collectorpb/collector.pb.go:	context "golang.org/x/net/context"

I tried re-generating those files but they kept importing x/net/context, I'm not super familiar with the build process here so I could take a pointer or two on how to fix this.

@iredelmeier
Copy link
Contributor

LGTM. We'll deal with the x/net/context in the fakes separately.

@iredelmeier iredelmeier reopened this Oct 22, 2018
@iredelmeier iredelmeier merged commit 65fac44 into lightstep:master Oct 22, 2018
@achille-roussel achille-roussel deleted the use-standard-context branch October 22, 2018 21:36
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.

2 participants