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 a check that primary and secondary use custom_format or not #1153

Merged

Conversation

ganmacs
Copy link
Member

@ganmacs ganmacs commented Aug 9, 2016

Not to warn about secondary plugin type if "standard" format is used in both of primary and secondary.

This PR closes #1150.

@tagomoris tagomoris added v0.14 enhancement Feature request or improve operations labels Aug 10, 2016
end

mock(d.instance.log).warn("secondary type should be same with primary one",
{ :primary=>d.instance.class.to_s, :secondary=>"Fluent::Plugin::Test2Output" })
Copy link
Member

Choose a reason for hiding this comment

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

Use json style hash literal instead of rocket operator.

Copy link
Member Author

@ganmacs ganmacs Aug 10, 2016

Choose a reason for hiding this comment

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

@tagomoris
Copy link
Member

test/test_output is tests for traditional (v0.12) output class. Adding tests in that file is not bad idea, but we need to write tests for current Fluent::Plugin::Output in test/plugin/test_output.rb.

@ganmacs ganmacs force-pushed the not-to-warn-about-secondary-plugin-type branch from 130cdb6 to e4e2204 Compare August 10, 2016 10:10
@ganmacs
Copy link
Member Author

ganmacs commented Aug 10, 2016

@tagomoris

test/test_output is tests for traditional (v0.12) output class.

I don't know that 😨
I add a new test for test/plugin/output.rb

e4e2204

@@ -477,6 +488,36 @@ def waiting(seconds)

i.stop; i.before_shutdown; i.shutdown; i.after_shutdown; i.close; i.terminate
end

test "Warn if primary type differents from secondary type and either primary or secondary has custom_format" do
Copy link
Member

Choose a reason for hiding this comment

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

"is different from"

Copy link
Member Author

Choose a reason for hiding this comment

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

@ganmacs ganmacs force-pushed the not-to-warn-about-secondary-plugin-type branch from e4e2204 to af8056f Compare August 15, 2016 11:32
assert_not_nil o.instance_variable_get(:@secondary)
end

test "don't warn if primary type is same as econdary type" do
Copy link
Member

Choose a reason for hiding this comment

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

"econdary" -> "secondary"

Copy link
Member Author

Choose a reason for hiding this comment

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

@ganmacs ganmacs force-pushed the not-to-warn-about-secondary-plugin-type branch 3 times, most recently from 7487104 to ec3847c Compare August 19, 2016 01:03
@ganmacs ganmacs force-pushed the not-to-warn-about-secondary-plugin-type branch from ec3847c to 971eaff Compare August 19, 2016 02:35
@tagomoris
Copy link
Member

LGTM. Thank you for contribution!

@tagomoris tagomoris merged commit 7194cb9 into fluent:master Aug 19, 2016
@ganmacs ganmacs deleted the not-to-warn-about-secondary-plugin-type branch November 28, 2019 00:07
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.

Not to warn about secondary plugin type if standard format is used in both of primary and secondary
2 participants