-
Notifications
You must be signed in to change notification settings - Fork 2
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
Rename Events to ActiveOutbox::Events to avoid conflicts. #22
Conversation
* Issue was raised that the Events const name was pretty common and this could cause conflicts. * Added false in const_defined so it won't lookup to ancestors.
@@ -10,16 +10,21 @@ module Outboxable | |||
*namespace, klass = name.underscore.upcase.split('/') | |||
namespace = namespace.reverse.join('.') | |||
|
|||
module_parent.const_set('Events', Module.new) unless module_parent.const_defined?('Events') | |||
module_parent.const_set('ActiveOutbox', Module.new) unless module_parent.const_defined?('ActiveOutbox', false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is module_parent
? is it configurable?
What I was saying is that any class created by this gem should go in ::ActiveOutbox
namespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would be the module name of where this concern is included. That we are not creating ourselves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm ok, I think we are good then
Issue was raised that the Events const name was pretty common and this could cause conflicts.
Added false in const_defined so it won't lookup to ancestors fixing Fix an issue with finding the Events module for namespaced models #21