Skip to content

Allow using different classes for each setting - #87

Merged
ledermann merged 4 commits into
ledermann:masterfrom
Jesus:master
Jul 30, 2018
Merged

Allow using different classes for each setting#87
ledermann merged 4 commits into
ledermann:masterfrom
Jesus:master

Conversation

@Jesus

@Jesus Jesus commented Jul 27, 2018

Copy link
Copy Markdown
Contributor

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

coveralls commented Jul 27, 2018

Copy link
Copy Markdown

Coverage Status

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

@Jesus

Jesus commented Jul 27, 2018

Copy link
Copy Markdown
Contributor Author

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

@Jesus

Jesus commented Jul 28, 2018

Copy link
Copy Markdown
Contributor Author

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

@Jesus

Jesus commented Jul 30, 2018

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Owner

Great work, thanks!

@ledermann

Copy link
Copy Markdown
Owner

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

@Jesus

Jesus commented Sep 6, 2018

Copy link
Copy Markdown
Contributor Author

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