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

Allow using different classes for each setting #87

Merged
merged 4 commits into from
Jul 30, 2018

Conversation

Jesus
Copy link
Contributor

@Jesus Jesus commented Jul 27, 2018

This update will allow us to set a custom :class_name for each setting. For example:

has_settings do |s|
  s.key :calendar # Will use the default RailsSettings::SettingObject
  s.key :info, :class_name => 'ProjectSettingObject'
end

This also accidentally solves #57 .

Jesús Burgos added 2 commits July 27, 2018 13:15
Prior to this, only one class name could be used for all settings within
a Rails model.
@coveralls
Copy link

coveralls commented Jul 27, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 29c1ddc on Jesus:master into 659160e on ledermann:master.

@Jesus
Copy link
Contributor Author

Jesus commented Jul 27, 2018

I'll take care of these build errors when I get a chance. Please ignore the PR for now.

@Jesus
Copy link
Contributor Author

Jesus commented Jul 28, 2018

Tests are finally passing, but I think I'll just add some more tests to cover persistence thoroughly.

@Jesus
Copy link
Contributor Author

Jesus commented Jul 30, 2018

Hi @ledermann, first off thanks for your work on this gem, it's been super handy for us.

Please let me know what you think of this PR, if you're willing to support the feature I propose it'd be great.

@ledermann
Copy link
Owner

Great work, thanks!

@ledermann
Copy link
Owner

Hm, the changes break my own application using RailsSettings, so I reverted them. Sadly I don't have the time to investigate...

@Jesus
Copy link
Contributor Author

Jesus commented Sep 6, 2018

What versions of Ruby & Rails are you using?

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

Successfully merging this pull request may close these issues.

3 participants