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

Change serialization syntax #867

Merged
merged 7 commits into from
Mar 29, 2016
Merged

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Mar 25, 2016

string as following

In previous version:
string_param1 string_value
string_param2 string
value

In this version:
string_param1 "string_value"
string_param2 "string\nvalue"

I don't change serialization format for other types.

See #522

string as following

In previous version:
   string_param1 string_value
   string_param2 string
value

In this version:
   string_param1 "string_value"
   string_param2 "string\nvalue"

I don't change serialization format for other types.
@okkez okkez changed the title Change serialization syntax [WIP] Change serialization syntax Mar 25, 2016
@okkez okkez changed the title [WIP] Change serialization syntax Change serialization syntax Mar 25, 2016
opts[:type]
end

def dump_value(k, v, indent, nindent)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason to assign out = "" at once and then do out << "..."?
It looks good enough just to return string value by if expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly.
I will fix this.

@okkez
Copy link
Contributor Author

okkez commented Mar 29, 2016

I've fixed and pushed.

@tagomoris tagomoris merged commit 06bf4bc into fluent:master Mar 29, 2016
@tagomoris
Copy link
Member

Merged! Thank you so much.

@okkez okkez deleted the change-dump-syntax branch April 8, 2016 01:16
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