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

Added inline bytes values #143

Open
wants to merge 1 commit into
base: v3.0-development
Choose a base branch
from
Open

Conversation

flippmoke
Copy link
Member

This is a possible solution to #142
/cc @joto @ericfischer

@joto
Copy link

joto commented Aug 28, 2019

I think we don't need a separate bytes_values, just reuse the string_values. All we need is the bit that says "interpret as bytes" vs. "interpret as string", the internal encoding is the same.

Also we might want to renumber the IDs while we can and make it id 0?

@flippmoke
Copy link
Member Author

@joto the weird part is that some protobuf encoder/decoders will enforce this to a string encoding, this isn't an issue in C++ but in other cases it is an issue. I suppose we could work around this by writing custom protobuf writers in each language but I could see this being a gotcha.

@joto
Copy link

joto commented Aug 29, 2019

@flippmoke Makes sense. Doesn't cost us anything really, so that's fine. Still think we should fix order though.

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