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

Remove redundant require #892

Merged
merged 7 commits into from
Apr 21, 2016
Merged

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Apr 11, 2016

This change suppresses following warnings(about 130 lines warnings):

lib/fluent/config/configure_proxy.rb:17: warning: loading in progress, circular require considered harmful
lib/fluent/config/types.rb:20: warning: loading in progress, circular require considered harmful
lib/fluent/event.rb:17: warning: loading in progress, circular require considered harmful
lib/fluent/filter.rb:19: warning: loading in progress, circular require considered harmful
lib/fluent/agent.rb:18: warning: loading in progress, circular require considered harmful
lib/fluent/output.rb:21: warning: loading in progress, circular require considered harmful
lib/fluent/root_agent.rb:21: warning: loading in progress, circular require considered harmful

@okkez
Copy link
Contributor Author

okkez commented Apr 11, 2016

See also #848

@@ -15,7 +15,6 @@
#

require 'fluent/configurable'
require 'fluent/engine'
Copy link
Member

Choose a reason for hiding this comment

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

Removing this require looks good, but Engine is referred below to get logger.
I think we need to fix Agent to get logger in constructor, and to pass it from Engine when RootAgent is created in engine.rb.

@tagomoris
Copy link
Member

@okkez I added some comments. Could you add some more fixes?

@okkez
Copy link
Contributor Author

okkez commented Apr 19, 2016

🆗 I will push fixes in this week.

okkez added 6 commits April 20, 2016 15:48
This change suppresses following warnings(about 130 lines warnings):

lib/fluent/config/configure_proxy.rb:17: warning: loading in progress, circular require considered harmful
lib/fluent/config/types.rb:20: warning: loading in progress, circular require considered harmful
lib/fluent/event.rb:17: warning: loading in progress, circular require considered harmful
lib/fluent/filter.rb:19: warning: loading in progress, circular require considered harmful
lib/fluent/agent.rb:18: warning: loading in progress, circular require considered harmful
lib/fluent/output.rb:21: warning: loading in progress, circular require considered harmful
lib/fluent/root_agent.rb:21: warning: loading in progress, circular require considered harmful
@okkez okkez force-pushed the remove-circular-require branch from 7fdfbe6 to 4d43a98 Compare April 20, 2016 06:50
@okkez
Copy link
Contributor Author

okkez commented Apr 20, 2016

I've pushed fixes for all comments.

@okkez
Copy link
Contributor Author

okkez commented Apr 20, 2016

Hmm, CI failed.

@okkez
Copy link
Contributor Author

okkez commented Apr 21, 2016

CI failed against cc73ce2.
Please retry, I think this failure is not related to my changes.

@tagomoris
Copy link
Member

Thank you very much! LGTM.

@tagomoris tagomoris merged commit 985a912 into fluent:master Apr 21, 2016
@okkez okkez deleted the remove-circular-require branch April 21, 2016 04:00
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