Skip to content

Allow SettingObject to respond to try getters.#74

Merged
ledermann merged 1 commit intoledermann:masterfrom
marcferna:master
Jun 24, 2016
Merged

Allow SettingObject to respond to try getters.#74
ledermann merged 1 commit intoledermann:masterfrom
marcferna:master

Conversation

@marcferna
Copy link
Contributor

@marcferna marcferna commented Jun 23, 2016

It was returning always nil when using the getter with try.
i.e user.try(:settings, :dashboard).try(:theme)

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 98.473% when pulling 4f6882e on marcferna:master into 1792dc3 on ledermann:master.

@coveralls
Copy link

coveralls commented Jun 23, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 4f6882e on marcferna:master into 1792dc3 on ledermann:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 98.473% when pulling 4f6882e on marcferna:master into 1792dc3 on ledermann:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 98.473% when pulling 4f6882e on marcferna:master into 1792dc3 on ledermann:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 98.473% when pulling 4f6882e on marcferna:master into 1792dc3 on ledermann:master.


def _setting?(method_name)
_target_class.default_settings[var.to_sym].keys.include?(
method_name.to_s.sub('=','')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO the sub can be removed here, because try should test for getter only.

@ledermann
Copy link
Owner

Nice change, thanks for contribution. I've made a small comment, maybe you can update the PR.

@marcferna
Copy link
Contributor Author

Changed

@coveralls
Copy link

coveralls commented Jun 24, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling d807ce7 on marcferna:master into 1792dc3 on ledermann:master.

@ledermann ledermann merged commit ab87bfc into ledermann:master Jun 24, 2016
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