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

Reorganize module options #50

Merged
merged 26 commits into from Jul 17, 2017
Merged

Reorganize module options #50

merged 26 commits into from Jul 17, 2017

Conversation

@shioyama
Copy link
Owner

shioyama commented Jul 8, 2017

Currently there is a lot of hard-coded dependencies between options-setting in
the Mobility::Attributes class and the various backend modules (cache,
fallbacks, etc.), whereas these should really be independent.

I'm going to try to restructure things so that modules can be defined in such a
way that Mobility::Attributes does not need to explicitly know anything about
them when including them from options.

@shioyama shioyama force-pushed the module_options branch 9 times, most recently from 25a1215 to 6f5ef60 Jul 10, 2017
@shioyama shioyama force-pushed the master branch from bbe819c to 9d50ef7 Jul 16, 2017
@shioyama shioyama force-pushed the module_options branch 2 times, most recently from 2e02538 to 85e4bca Jul 16, 2017
@shioyama
Copy link
Owner Author

shioyama commented Jul 17, 2017

This change adds extracts hard-coded dependencies in the Mobility::Attributes class to the different "option modules" (cache, fallbacks, etc) so that the order and set of modules to include can be entirely customized.

Two new configuration options can be used to do this:

Mobility.option_modules         
#=> {
#   :cache => Mobility::Backend::Cache,          
#   :dirty => Mobility::Backend::Dirty,             
#   :fallbacks => Mobility::Backend::Fallbacks,     
#   :presence => Mobility::Backend::Presence,       
#   :default => Mobility::Backend::Default,         
#   :fallthrough_accessors => Mobility::FallthroughAccessors,                                      
#   :locale_accessors => Mobility::LocaleAccessors
# }

This is a hash of option keys and constants (classes or modules). The only requirement of the classes used as hash values is that they have a singleton method apply which takes an instance if Attributes and an option value (typically a boolean) and then does something with the Attributes instance, typically including a module into attributes.backend_class or into attributes itself.

This hash is iterated over here when initializing an Attributes module, taking the option value for the key and passing it and the attributes module itself to the option module (e.g. Mobility::Backend::Class.apply(attributes, true), if cache: true was passed to translates).

Mobility.default_options
#=> {:cache=>true, :dirty=>false, :fallbacks=>nil, :presence=>true, :default=>nil}

The other configuration option is Mobility.default_options. This gets merged into the options passed into translates, thus allowing you to set a hash of defaults to be used across all your models.

@shioyama shioyama force-pushed the module_options branch from 85e4bca to 0e6d797 Jul 17, 2017
@shioyama shioyama merged commit 0e6d797 into master Jul 17, 2017
3 checks passed
3 checks passed
codeclimate 2 fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@shioyama shioyama deleted the module_options branch Jul 17, 2017
@valscion
Copy link
Contributor

valscion commented Oct 25, 2017

Thank you for creating these detailed pull requests, and elaborating your thoughts here! Even though it might feel like no-one is reading these, I for one am grateful to be able to follow along how this gem evolves, and how you think through these changes.

🙇

@shioyama
Copy link
Owner Author

shioyama commented Oct 25, 2017

Thanks! That's great to hear. I was mainly doing these PRs as mental notes to myself, with the idea that anybody who was interested could follow along, but very glad to hear that they are useful to you and others. Really appreciate that you took the time to express that here 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.