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

Deprecate alias_method_chain in favour of Module#prepend #19434

Merged
merged 1 commit into from Mar 22, 2015
Merged

Deprecate alias_method_chain in favour of Module#prepend #19434

merged 1 commit into from Mar 22, 2015

Conversation

kirs
Copy link
Member

@kirs kirs commented Mar 20, 2015

…as discussed #19413
@rafaelfranca

@rafaelfranca
Copy link
Member

We are still using it internally right? Lets remove all the calls before deprecating.

@@ -21,6 +21,8 @@ class Module
#
# so you can safely chain foo, foo?, foo! and/or foo= with the same feature.
def alias_method_chain(target, feature)
ActiveSupport::Deprecation.warn("alias_method_chain is deprecated. Please, use Module#prepend instead.")
Copy link
Contributor

Choose a reason for hiding this comment

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

alias_method_chain is usually used to extend an existing method and call the original. This deprecation method leaves the last part implicit. Let's mention you can access the original method using super.

@kirs
Copy link
Member Author

kirs commented Mar 22, 2015

@rafaelfranca done.

Now there still 2 places where alias_method_chain is called:

@rafaelfranca
Copy link
Member

@kirs in Rage core_ext lets remove the usage of alias_method_chain with its implementation, so we don't get deprecation warnings in our test, and add a TODO note to remove it as soon the bug is fixed.

@kirs
Copy link
Member Author

kirs commented Mar 22, 2015

@rafaelfranca done!

rafaelfranca added a commit that referenced this pull request Mar 22, 2015
Deprecate alias_method_chain in favour of Module#prepend
@rafaelfranca rafaelfranca merged commit 4ba9c55 into rails:master Mar 22, 2015
@dhh
Copy link
Member

dhh commented Mar 24, 2015

I don't see how this is a replacement? Prepend operates on the whole module, requires the method that's beind prepended to call super. If we would have gotten the proposed features of before/after/around, that would have been a straight up replacement. This isn't.

@rafaelfranca
Copy link
Member

@dhh the method that call super is the method that is extending, the original method don't need to take care of it. Could you show an example where prepend doesn't work?

@rafaelfranca
Copy link
Member

When we proposed prepend to Ruby core was with the idea of being a complete replacement for alias_method_chain. I may be missing a case where prepend doesn't work, but I always could replace alias_method_chain with prepend without touching the original method.

@dhh
Copy link
Member

dhh commented Mar 24, 2015

Ah, okay, maybe I missed that. I’ll have another look.

On Mar 24, 2015, at 10:37 AM, Rafael Mendonça França notifications@github.com wrote:

@dhh https://github.com/dhh the method that call super is the method that is extending, the original method don't need to take care of it. Could you show an example where prepend doesn't work?


Reply to this email directly or view it on GitHub #19434 (comment).

@rafaelfranca
Copy link
Member

Right. Let me know if you find a case, so we can remove the deprecation.

@@ -1,3 +1,7 @@
* Deprecate `alias_method_chain` in favour of `Module#prepend` introduced in Ruby 2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

@kirs Rails tries to stick to american english, so next time prefer favor over favour etc. 😄

GabrielSandoval pushed a commit to GabrielSandoval/active_model_serializers_contrib that referenced this pull request Mar 13, 2018
- #alias_method_chain has been deprecated in Rails 5
See: rails/rails#19434
GabrielSandoval pushed a commit to GabrielSandoval/active_model_serializers_contrib that referenced this pull request Mar 21, 2018
- #alias_method_chain has been deprecated in Rails 5
See: rails/rails#19434
kamipo added a commit that referenced this pull request Jul 31, 2018
"active_support/core_ext/module/aliasing" is no longer used since
#19434.
lizconlan added a commit to mysociety/whatdotheyknow-theme that referenced this pull request Aug 21, 2019
alias_method_chain was deprecated in Rails 5.0 and removed in 5.1

rails/rails#19434

As the method we're overriding here is a class method rather than an
instance method, the suggested fix of using Module#prepend doesn't seem
to work so have switched to this simpler - albeit clumsy - fix instead
of copying in the original method (in lieu of the renamed method
technique we were relying on before) and adding overriding the original
class method with `alias_method` instead

Inspired by CachedResource's solution:
https://github.com/MeisterLabs/cached_resource/blob/4a16812812ec427784d95c803b5d15a3db6d8e42/lib/cached_resource/caching.rb
lizconlan added a commit to mysociety/whatdotheyknow-theme that referenced this pull request Aug 21, 2019
alias_method_chain was deprecated in Rails 5.0 and removed in 5.1

rails/rails#19434

As the method we're overriding here is a class method rather than an
instance method, the suggested fix of using Module#prepend doesn't seem
to work so have switched to this simpler - albeit clumsy - fix instead
of copying in the original method (in lieu of the renamed method
technique we were relying on before) and adding overriding the original
class method with `alias_method` instead

Inspired by CachedResource's solution:
https://github.com/MeisterLabs/cached_resource/blob/4a16812812ec427784d95c803b5d15a3db6d8e42/lib/cached_resource/caching.rb
mysociety-pusher pushed a commit to mysociety/whatdotheyknow-theme that referenced this pull request Oct 29, 2019
alias_method_chain was deprecated in Rails 5.0 and removed in 5.1 [1]

Class methods are spilt between two modules because of a RSpec mock
issue [2]. We're using:
  - include: to allow the `alert_survey` method to still be mocked in
    our specs
  - prepend: to allows us to override a method and call `super` to run
    the original implementation of the method in Alaveteli core.

[1] rails/rails#19434
[2] rspec/rspec-mocks#1213
gbp pushed a commit to mysociety/whatdotheyknow-theme that referenced this pull request Nov 1, 2019
alias_method_chain was deprecated in Rails 5.0 and removed in 5.1 [1]

Class methods are spilt between two modules because of a RSpec mock
issue [2]. We're using:
  - include: to allow the `alert_survey` method to still be mocked in
    our specs
  - prepend: to allows us to override a method and call `super` to run
    the original implementation of the method in Alaveteli core.

[1] rails/rails#19434
[2] rspec/rspec-mocks#1213
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

4 participants