Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

ActiveSupport::OrderedOptions calls to_sym redundantly

#6483

There's no need to call to_sym on the matched method name. The [] method already does this.

Reported by Ben Orenstein · February 28th, 2011 @ 01:31 AM

State: committed
Milestone: none
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • State changed from new to verified
    • Assigned user set to Santiago Pastorino
    • Importance changed from to Low

    Verified. Applies cleanly to master and test pass. test_looping in ordered_options_test (https://github.com/rails/rails/blob/master/activesupport/test/order...) tests if strings are converted to symbols.

    Nice one, Ben! :)

    February 28th, 2011 @ 02:18 PM

  2. Ben Orenstein
    Ben Orenstein

    Thanks Jeff!

    If applied, this will by my first non-documentation, non-test-code patch :)

    February 28th, 2011 @ 03:16 PM

  3. Josh Kalderimis
    Josh Kalderimis

    Hey Ben,

    Generally the patch is good but you have also removed the require "ordered_hash", is there a particular reason?

    The rule of thumb is if you only require a subset of ActiveSupport into your project, like ordered_options, everything that is required for the requires should also be setup and required correctly as well, eg. ordered_hash.

    If you can fix this and include a new updated patch it would be appreciated.

    Thanks,

    Josh

    February 28th, 2011 @ 03:39 PM

  4. Ben Orenstein
    Ben Orenstein

    Oops. That change snuck in there accidentally.

    Thanks for the catch, Josh.

    This new patch fixes that.

    February 28th, 2011 @ 03:56 PM

  5. Josh Kalderimis
    Josh Kalderimis

    Hey Ben,

    Thanks for the patch, looks good to me. +1.

    Santiago, can you please take a look and do your magic :)

    Thanks again Ben!

    Josh

    February 28th, 2011 @ 03:57 PM

  6. Repository
    Repository
    • State changed from verified to committed

    February 28th, 2011 @ 06:44 PM