metaverse/truss

Latest versions of protoc/proto-gen-go breaks code generation

matthewhartstonge opened this issue · 5 comments

matthewhartstonge commented

I've just brought up a new laptop and pulled down truss and the latest protoc-gen-go (1.1.0) requires protoc 3.5.1. Also leaves wonderful internal fields prefixed with XXX_ all over the place when generating.

I've been able to work out what deps need to be updated and add a Truss patch to remove the XXX fields, but just got to the point where Go-Kit v0.5.0 needs a 2 line patch due to renamed GRPC functions... (fixed in v0.6.0+)

So for now, it might be nice to update the readme to say that protoc <=3.4 and protoc-gen-go == 1.0.0 are required.

zaquestion commented

@matthewhartstonge thanks again that saves me some debugging, hopefully we can just get everything update to the latest.

  • 👍1
matthewhartstonge commented

@zaquestion are you suggesting moving Truss to support kit v0.7.0?

re: protobuf, @pauln suggested having a look into https://github.com/gogo/protobuf which seems to be a pretty solid fork of protobuf used by pretty big projects (kube, dropbox, NATS). The reason for this is that protoc-gen-gogofaster which generates code potentially without the XXX_ fields.

Actually, scrap that, as go-kit has a direct dependency on google.golang.org/grpc 🙃

zaquestion commented

@matthewhartstonge yeah, in general I'd like to see truss support the latest go-kit and keep up to date with the changing landscape of dependencies.

gogoprotobuf looks really nice, I'd love to see truss support it in addition to the google/protobuf, or even evaluate switching to it wholesale. I didn't know there were some really projects using it, but I'm not surprised! It seems to have a better idea about what Go needs out of protobuf.

One users did some work here, that seems to have gotten merged into master on their fork master...applinskinner:gogoprotobuf

zaquestion commented

@matthewhartstonge The build should be working now and we've updated the version of gokit to 0.7.0, likewise the latest protoc-gen-go should work with protoc 3.5.1. In doing so we've dropped support for generated documentation -- we don't believe people to be getting value out of it and are ultimately looking forward at using swagger for that.

Let me know how things are looking on your side

  • ❤️1
matthewhartstonge commented

Thank you for the amazingly quick turn around on this!
Swagger makes sense! Swagger definitions are pretty awesome due the client code generation tools.
I'll look at updating our downstream this week and aim to look at sending in a PR for #233 😄 👌

  • ❤️1