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 migration flag for users using old JSON format. #55

Merged
merged 1 commit into from
Apr 14, 2016

Conversation

haberman
Copy link
Member

This is intended for Ruby users who may currently be using JSON format. The Ruby JSON format was written before we realized that proto3 JSON would be camelCasing field names, so it always used original field names. The most recent upb code fixes this problem, but this could cause problems for any Ruby users who are currently using JSON.

These Ruby users should migrate, but it may be difficult to do all at once. So we are providing these flags for now to make the migration easier.

In the longer term, if Ruby users need compatibility with existing payloads, they should use the json_name option in their .proto files to specify the old names. Unfortunately proto3 Ruby doesn't support json_name yet. We will add support for it before we remove these flags.

@haberman
Copy link
Member Author

Review to @scwhittle.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 79.326% when pulling 1019f76 on haberman:jsonflag into 458077c on google:master.

len = upb_fielddef_getjsonname(f, ret->ptr, ret->len);
UPB_ASSERT_VAR(len, len == ret->len);
ret->len--; /* NULL */
if (getenv("UPB_JSON_WRITE_LEGACY_FIELD_NAMES")) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a performance concern? You could cache somewhere instead of looking up the env variable repeatedly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code runs at handler creation time, not at print time.

@scwhittle
Copy link

LGTM

@haberman haberman merged commit 534b5c9 into protocolbuffers:master Apr 14, 2016
@haberman haberman deleted the jsonflag branch December 1, 2018 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants