src-d / proteus

Generate .proto files from Go source code.
https://blog.sourced.tech/post/proteus/
MIT License
735 stars 70 forks source link

Problem with type `int` #40

Closed erizocosmico closed 7 years ago

erizocosmico commented 7 years ago

Problem

There is no int in protobuf, only int32 and int64. Then, when we generate the .proto file we have to choose between one of them. Right now, the choice is int32.

Problem is, when doing the gogoproto step, it takes into account the proto file, and not the Go source code, and thus what it thinks is that the type is an int32 instead of int.

Steps to reproduce:

//proteus:generate type Foo struct { A int }



## Fix

* Set custom type on proto when the type has no direct translation in protobuf
* Change the transation of `int` to `int64` and `uint` to `uint64` in protobuf

## Caveats

* It probably happens as well with `time.Duration` and all the types that don't have a clear direct mapping (all other ints besides signed and unsigned `int32` and `int64`)
Serabe commented 7 years ago

We can error if we see type int that user should change it to int32 or int64.

erizocosmico commented 7 years ago

This was fixed, right?

Serabe commented 7 years ago

Yes, but since we are using services it is not closed automagically.

dmitshur commented 7 years ago

What are you referring to with services?

Serabe commented 7 years ago

services branch.

dmitshur commented 7 years ago

I see.

I'm not familiar with the details, so this suggestion might be off base, but have you considered making that branch the default branch of this repository? If you're using it that way.

Then, when a PR merged into services branch says "fixes" some issue, it will get closed by github.

It would affect go get, so perhaps you don't want to do it.

erizocosmico commented 7 years ago

We were using services branch as a "development" branch until all the pending issues and the gRPC services implementation were finished. It should be finished today or tomorrow and we'll merge it into master.