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

Replace Config::Options#delegate to simple method definitions #185

Merged
merged 10 commits into from
Nov 7, 2017
Merged

Replace Config::Options#delegate to simple method definitions #185

merged 10 commits into from
Nov 7, 2017

Conversation

yhatt
Copy link
Contributor

@yhatt yhatt commented Nov 6, 2017

Fix #184.

Config::Options#delegate is using ActiveSupport's #delegate implicitly. So it would raise
NoMethodError with running in not rails environment. e.g. Padrino, Sinatra and pure Ruby.

It could fix it by replacing to simple method definitions.

Yuki Hattori added 2 commits November 6, 2017 11:36
delegate method is required ActiveSupport. So it would raise
NoMethodError with running in not rails environment.
@pkuczynski
Copy link
Member

Thanks for the fix. Could you please add a test for it?

@yhatt
Copy link
Contributor Author

yhatt commented Nov 6, 2017

Well, we need to add not-rails environment to test it. I would try it by Sinatra.

@pkuczynski
Copy link
Member

Cool, thanks! That should be easily possible with Appraisals

Copy link
Contributor Author

@yhatt yhatt left a comment

Choose a reason for hiding this comment

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

Ready to test with Sinatra! I confirmed to fail sinatra test with Config::Options#key? and #has_key? provided by ActiveSupport's #delegate.

spec_helper.rb is updated to support sinatra, but CodeClimate tells an issue...


def fixture_path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For preventing to fail current specs, I defined fixture_path manually in sinatra test. fixture_path is provided by rspec-rails.

@yhatt
Copy link
Contributor Author

yhatt commented Nov 7, 2017

Yay, checks passed! Adding sinatra test was a bit harder than I thought 😅

@pkuczynski
Copy link
Member

You did a great job! :) I am waiting for Travis to report the build status and we can merge it in :)

@pkuczynski
Copy link
Member

All good! Merging in and releasing...

@pkuczynski pkuczynski merged commit b35f4af into rubyconfig:master Nov 7, 2017
@pkuczynski pkuczynski added this to the 1.6.1 milestone Nov 7, 2017
@pkuczynski pkuczynski added the bug label Nov 7, 2017
@yhatt yhatt deleted the delegate-in-not-rails-env branch November 9, 2017 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Undefined method 'delegate' when used with Padrino 0.14
2 participants