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

wire.proto not in repo? #19

Open
nornagon opened this issue Feb 19, 2017 · 3 comments
Open

wire.proto not in repo? #19

nornagon opened this issue Feb 19, 2017 · 3 comments

Comments

@nornagon
Copy link

It looks like the generated file is in the repo, but not the Proto source?

@bhs
Copy link
Contributor

bhs commented Feb 19, 2017

@nornagon that's correct (I'm not suggesting it's better this way!). The file can be found here: https://github.com/opentracing/basictracer-go/blob/master/wire/wire.proto

The "right" (or "righter") thing would be to factor out the protos into something like a basictracer-common repo and add said repo as a submodule here and in other basictracer repos. If you want to take this on, I can help with the mechanics.

@nornagon
Copy link
Author

Ah, I see, that makes sense. For xray-python-opentracing I ended up rewriting the binary format to not depend on protobuf (since I needed to serialize trace ids as strings, and the format is pretty simple). I get that pb makes more sense for language interop though.

I'd be happy to take on that refactor. The only bit I wouldn't be able to make a PR for would be creating the new repo.

@bhs
Copy link
Contributor

bhs commented Feb 22, 2017

@nornagon I think it's also permissible to ignore the binary format while you work out the kinks of the rest of the lib... IIRC the basictracer-python stuff even has some weird code to avoid the protobuf imports entirely since they caused some packaging issues in production codebases.

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

No branches or pull requests

2 participants