Remove methods that are never invoked #21616
Conversation
|
This PR is simply confirming that, from now on, ActiveSupport::Callbacks has a default terminator, and therefore we don't need to check whether a terminator exists or not. This does not close the discussion opened by @repinel at #21218 where he correctly points out that what the default terminator does might not be what we want in Rails 5. Independently of that, a default terminator exists, which justifies the purpose of this PR. |
claudiob
force-pushed
the
fix-21122
branch
2 times, most recently
from
2cadaaf
to
eebf4fc
Compare
Fixes rails#21122 - does not change any current behavior; simply reflects the fact that two conditions of the if/else statement are never reached. The reason is rails#17227 which adds a default terminator to AS::Callbacks. Therefore, even callback chains that do not define a terminator now have a terminator, and `chain_config.key?(:terminator)` is always true. Of course, if no terminator was defined, then we want this new default terminator not to do anything special. What the terminator actually does (or should do) is discussed in rails#21218 but the simple fact that a default terminator exists makes this current PR valid. *Note* that the conditional/simple methods have not been removed in AS::Conditionals::Filter::After because of `:skip_after_callbacks_if_terminated` which lets a user decide **not** to skip after callbacks even if the chain was terminated.
|
Also note that the conditional/simple methods have not been removed in AS::Conditionals::Filter::After because of |
|
I don't understand why the Travis CI badge for this PR is not showing, but all the tests for this PR are passing. |
|
Looks good 👍 |
Remove methods that are never invoked
Fixes #21122 - does not change any current behavior; simply reflects
the fact that two conditions of the if/else statement are never reached.
The reason is #17227 which adds a default terminator to AS::Callbacks.
Therefore, even callback chains that do not define a terminator now
have a terminator, and
chain_config.key?(:terminator)is always true.Of course, if no terminator was defined, then we want this new default
terminator not to do anything special. What the terminator actually does
(or should do) is discussed in #21218 but the simple fact that a default
terminator exists makes this current PR valid.