-
-
Notifications
You must be signed in to change notification settings - Fork 121
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
Improve decorators #159
Improve decorators #159
Conversation
Based on the discussion with @spaghetticode and @kennyadsl, I've decided to use this PR just for removing class_eval from the existing decorators. The decision on a new pattern for organizing "decorators" in Solidus should be discussed and settled in the issue on the main repo. |
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.
I'm ok with just removing class_eval here but I think there's a typo in a file name.
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.
Thanks @jacobherrington!
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.
👍 looks good to me!
I'm not completely confident that testing would catch all of the problems that could arise from this, but it is passing tests...
Open to feedback on whether or not this change is worth it, I think it is.