-
Notifications
You must be signed in to change notification settings - Fork 807
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
Add documentation on the autogenerated methods #198
Comments
I was thinking something like: // Unmarshaler unmarshals a binary (protobuf?) representation of itself.
// It will always pass t.Size() bytes as its argument
// etc.
type Unmarshaler interface {
Unmarshal([]byte) error
} |
Minimal comments on public APIs are welcome. Maybe you would like to make a pull request? I think a generated slice of zero bytes will only be passed if this is what your NewPopulated function returned? But maybe I am wrong? Checking whether something implements the Unmarshaler or UnmarshalBinary takes time and slows down unmarshaling. Don't get me wrong cusomtype definitely has its issues. Its the second hardest feature to maintain and its not even designed correctly. If only proto3 had added a 128bit type with time.Time and time.Duration then no one would even use customtype in my opinion. Most people just want [16]byte for IPv6 and Uuid. |
I could try making a PR. Thing is, I'm not quite sure what documentation to write (thus this issue). Ahhh I wish proto3 had a 128 bit type. Would make life so much easier :) Here's are the methods: https://gist.github.com/EricLagergren/f798984c161dbdb6428372ee5d990963 I receive a whole bunch of:
|
Actually, this is causing it:
|
Fouuuuuund the issue. I'll create a new one for posterity. |
Like you say what documentation to write can be quite tricky. Someone would have to go through the code generator and look at all the options and try to make a concise sentence or two to document the generated method. |
I'm using custom types and have run into some issues.
For example, I've a UUID type (
type UUID [16]byte
). I added theUnmarshal
method which calls the already-existingUnmarshalBinary
method. However,UnmarshalBinary
checks to make sure the slice passed to it is 16 bytes long.In the generated tests a buffer with a length of 0 is passed, causing errors.
It'd be nice if there was some documentation so people like me who want to abuse protobufs a bit have some sort of guarantees about how the methods work.
(Aside: it'd be lovely if UnmarshalBinary could work in lieu of Unmarshal, similarly with UnmarshalJSON and UnmarshalText :-)
Also, thanks for this library. It's terrific.
The text was updated successfully, but these errors were encountered: