This project is archived and is in readonly mode.

#863 ✓wontfix
Clemens Kofler

ActiveRecord::Base refactoring

Reported by Clemens Kofler | August 19th, 2008 @ 11:30 PM | in 2.x

While checking out the terrain for another bigger fix, I stumbled across a little inconsistency in AR::Base.

In the SQL builder methods, add_joins!, add_limit! and add_lock! currently take the full options hash whereas their siblings, add_conditions!, add_order! and add_group! take only their relevant attribute. While I see why add_limit! needs to do that (it may take both, :limit and :offset), add_lock! and add_joins! only use their respective options and no other one.

I therefore propose the attached patch to always (and therefore consistently) pass in the full options hash. Although it's only an internally used, private method I think that consistency is still important.

All existing tests pass (apart from the two tests that seem to be failing anyways in the current Edge).

Comments and changes to this ticket

  • Pratik

    Pratik August 20th, 2008 @ 12:48 AM

    • State changed from “new” to “wontfix”

    Don't really see any benefits of this. Maybe this could be done whenever those methods need to access other option values.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages