Remove active attr #48
Conversation
| @@ -1,5 +1,33 @@ | |||
| module ActiveRemote | |||
| module Attributes | |||
| extend ActiveSupport::Concern | |||
| include ActiveModel::AttributeMethods | |||
There was a problem hiding this comment.
Does this work? I can see specs are passing, but I'd expect this to have to be included in the base class.
There was a problem hiding this comment.
This is a direct holdover from active_attr, and the ability it has to be used piecemeal.
https://github.com/cgriego/active_attr/blob/master/lib/active_attr/attributes.rb#L24
Certainly will be refactoring this in the next pull request, since we do not need the ability to use pieces.
| # Person.attribute_names | ||
| # | ||
| def attribute_names | ||
| attributes.keys |
There was a problem hiding this comment.
This is what I was thinking we could check instead of respond_to when we get to optimizations.
There was a problem hiding this comment.
I updated the 3.0 roadmap to make sure we addres any performance issues with the write_attribute method.
lib/active_remote/base.rb
Outdated
| include ActiveAttr::QueryAttributes | ||
| include ActiveAttr::Serialization | ||
| extend ::ActiveModel::Naming | ||
| include ActiveModel::Conversion |
There was a problem hiding this comment.
Let's keep scope resolution consistent. I'm good either way.
There was a problem hiding this comment.
I was planning on fixing this to be explicit for the entire project later...I can do that now though?
There was a problem hiding this comment.
Mainly wanted to make sure it was on your radar.
|
|
|
|
Over time, this project has slowly started to override significant parts of active_attr. This has gotten to a point where we have diverged significantly from active_attr and it is now very difficult to understand our interactions with active_attr.
This pulls in the remaining classes from active_attr and drops the dependency so that we can make do some significant refactoring in the 3.0 release.