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

Swaps out alias_method_chain for Module#prepend #88

Merged
merged 1 commit into from Sep 13, 2016

Conversation

nikz
Copy link
Contributor

@nikz nikz commented Sep 9, 2016

(in Ruby versions that support it)

@nikz nikz self-assigned this Sep 9, 2016
@armstrjare
Copy link
Contributor

Don't see the point in maintaining two separate code paths for different versions just for this simple case. I mean, alias_method_chain works in both right now - right? Unless we want to remove the ActiveSupport dependency.

The other option is to bump the gem requirements to 2.0+?

@nikz
Copy link
Contributor Author

nikz commented Sep 13, 2016

Rails 5 has a deprecation warning on 'alias_method_chain' and this avoids
that.

Happy to drop 1.9 support though.

Realistically we should backport this into the OAuth gem.
On Tue, 13 Sep 2016 at 08:20, Jared Armstrong notifications@github.com
wrote:

Don't see the point in maintaining two separate code paths for different
versions just for this simple case. I mean, alias_method_chain works in
both right now - right? Unless we want to remove the ActiveSupport
dependency.

The other option is to bump the gem requirements to 2.0+?


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#88 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAZStl9ksoVpQMaMbIvOjRkeyQlTC7gks5qpk67gaJpZM4J5f_G
.

@armstrjare
Copy link
Contributor

Ah, right - this is for the deprecation warning.

Let's apply this for now, but remove when we remove support for 1.9 later.

@armstrjare armstrjare merged commit b46ef54 into master Sep 13, 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.

None yet

2 participants