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

Add formatted_to_msgpack_binary? to Output plugin API. fix #1538 #1547

Merged
merged 2 commits into from
Apr 19, 2017

Conversation

repeatedly
Copy link
Member

No description provided.

@repeatedly repeatedly requested a review from mururu April 19, 2017 17:43
@repeatedly repeatedly added enhancement Feature request or improve operations v0.14 labels Apr 19, 2017
@repeatedly repeatedly self-assigned this Apr 19, 2017
@repeatedly
Copy link
Member Author

@mururu Could you also check this patch?

@mururu
Copy link
Member

mururu commented Apr 19, 2017

@repeatedly sure.

test 'plugin using custom format cannot iterate chunk in #write' do
data('formatted_to_msgpack_binary?' => :custom,
'formatted_to_msgpack_binary' => :old_custom)
test 'plugin using custom format cannot iterate chunk in #write' do |out_type|
Copy link
Member

Choose a reason for hiding this comment

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

This testcase doesn't verify this change because @format_type_is_msgpack.call always returns false regardless of overriding #formatted_to_msgpack_binary?.

@mururu
Copy link
Member

mururu commented Apr 19, 2017

This test case is better to add tests for :custom and :old_custom?
https://github.com/fluent/fluentd/pull/1547/files#diff-9b239225006e1bf484998d3e72b28704R338

@repeatedly
Copy link
Member Author

You are right. I just changed test case.

@mururu
Copy link
Member

mururu commented Apr 19, 2017

LGTM

@repeatedly repeatedly merged commit 4dddd63 into master Apr 19, 2017
@repeatedly
Copy link
Member Author

Thx!

@repeatedly repeatedly deleted the make-formatted_to_msgpack_binary-rubyish branch April 19, 2017 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants