Refactored method SpawnMethods#apply_finder_options #5776

wants to merge 6 commits into


None yet

4 participants


I refactored the SpawnMethods#apply_finder_options a bit. Two things I have done:

  • The options :conditions, :include, :extend cannot map directly on a method inRelation::QueryMethods. I made the subsitution explicit instead of filtering out the keys from the loop and performing the methods one by one after.
  • Extracted the filtering of the invalid and empty options for readability.

All ActiveRecord and Rails tests pass.

I'd like to deprecate the three finder-options: :conditions, :include, :extend. This would mean a lot of (fairly simple) changes in code, test and documentation. Let me know what you think of this - I could add it to the PR.

Ruby on Rails member

This no longer cleanly merges, and will need a rebase.


Rebased the issue-branch onto master - ran the tests and they pass.

Something is off though, Github is showing the last three rails/master commits from today as part of my pull request. To be sure: commit 5b46026 contains the refactoring for this pull request.

Ruby on Rails member

Finder options have already been deprecated and moved from master to rails/active_record_deprecated_finders, so I believe this is no longer valid. If you want to do any work on top of that, you can go ahead and send pull requests to that repo. Just a note that it'll be only maintained until Rails 5, as the Readme says.

Thanks for your work on that anyway :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment