-
Notifications
You must be signed in to change notification settings - Fork 178
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
Feature for field method, passing a type option will accepts any active record sql types and type casts the value when it is set. #84
base: master
Are you sure you want to change the base?
Conversation
will accepts any active record sql types and type casts the value when it is set.
Thanks! That's a great idea. So far active_hash doesn't depend on ActiveRecord (you can optionally include modules that depend on ActiveRecord, but core ActiveHash::Base doesn't know about AR), and I'd like to keep it that way. Your patch would require us to create that dependency as it stands. I would be OK relying on Virtus (https://github.com/solnic/virtus) which is a much smaller library. Are you familiar with Virtus? I've also been considering moving all the active_record-related methods to a different gem. Any thoughts? |
I have been looking into the Virtus code base and found On Sat, Feb 8, 2014 at 8:29 PM, Jeff Dean [email protected] wrote:
|
Yes, I think defaults should also be coerced. If performance ends up being an issue, we should be able to solve it pretty easily. Thanks!
|
Looks like the virtus gem is failing under ruby 1.8.7 |
Maybe it's time to release a version 2 of active hash that only supports ruby 1.9+. Or maybe we could make the active record-esque type coercion a module, so it solves your case, but doesn't force us to add a hard dependency. Any strong opinions?
|
I do like making it into a module you could give people a choice of which style they want to use either AR or Virtus, but that would add complexity. I am not sure if it's a problem now with AR but if it's kept up to date it will not support 1.8.7. Keeping AR at an older vesrion will not solve the case of matching the current AR api. |
I wonder if this would be failing under 1.8.7 if the Not sure if they would be receptive to using the old syntax. I know 1.9 has been end of life'd, but I'd imagine 1.8.7 may be around for a little while longer. |
I think it's probably reasonable to stop supporting 1.8.7 now with active hash. I think I'd like to close the open issues, release one more clean version, then roll this into a version 2, dropping support for 1.8.7.
|
I just released 1.3.0, which included all the bug fixes I wanted to include before making any major changes. Unless there are any objections, I think I'll drop 1.8.7 support, and release a 2.0 with these changes. |
@zeisler - would you mind squashing those commits? |
This feature is helpful because I use ActiveHash for mock objects in my tests and this would allow them to more resemble ActiveRecord's functionality.