Forum: Rails-core (closed, excessive spam) ActiveRecord::Base#attributes

81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-26 14:11
(Received via mailing list)
I noticed that [8863] remove the :except and :only options from
ActiveRecord::Base#attributes (and it makes perfect since not to
duplicate Hash#except/slice in there). #attributes still takes an
options parameter, but just ignores it, which could lead to a tricky
to diagnose error further down the line. Could a deprecation notice
not be issued (or make attributes not take an options parameter to
that ArgumentError is raised if people try) to make this easier to spot?

Fred
0f630dfa170eb15cb9bd041783b6551f?d=identicon&s=25 Tarmo Tänav (Guest)
on 2008-04-26 15:50
(Received via mailing list)
On L, 2008-04-26 at 13:10 +0100, Frederick Cheung wrote:
> I noticed that [8863] remove the :except and :only options from
> ActiveRecord::Base#attributes (and it makes perfect since not to
> duplicate Hash#except/slice in there). #attributes still takes an
> options parameter, but just ignores it, which could lead to a tricky
> to diagnose error further down the line. Could a deprecation notice
> not be issued (or make attributes not take an options parameter to
> that ArgumentError is raised if people try) to make this easier to spot?

I pointed this issue out in #rails-core but noone responded.

The strip_attributes[1] plugin is the only place I've seen using
this feature; with [8863] the plugin would actually misbehave
silently (stripping all attributes).

I think the option should be removed, and an appropriate upgrade
strategy described in the release notes.

An alternative would be to do something like this:
def attributes(options=nil)
  if options
    raise ArgumentError, 'The options argument is no longer accepted,
please use Hash#except or Hash#slice'
..
But I'd prefer the fiest approach as the simple fact that
the feature was removed suggests that it is not commonly used
and as such does not need special attention.


[1] http://rubyforge.org/projects/stripattributes
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-26 16:10
(Received via mailing list)
On 26 Apr 2008, at 14:48, Tarmo Tänav wrote:

>
> I pointed this issue out in #rails-core but noone responded.
>
> The strip_attributes[1] plugin is the only place I've seen using
> this feature; with [8863] the plugin would actually misbehave
> silently (stripping all attributes).
>
It's the silent stuff which is bad. Spent a good 15 minutes scratching
my head while trying out one of my apps
> I think the option should be removed, and an appropriate upgrade
> strategy described in the release notes.

Sounds like a plan to me. I'm happy to knock up the (completely
trivial) patch.

Fred

> An alternative would be to do something like this:
> def attributes(options=nil)
>  if options
>    raise ArgumentError, 'The options argument is no longer accepted,
> please use Hash#except or Hash#slice'
> ..
> But I'd prefer the fiest approach as the simple fact that
> the feature was removed suggests that it is not commonly used
> and as such does not need special attention.
yes, perhaps a little over the top.
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2008-04-27 00:28
(Received via mailing list)
>  Sounds like a plan to me. I'm happy to knock up the (completely
>  trivial) patch.

Sounds great, we should also add a deprecation warning to 2-0-stable,
care to whip up both patches? ;)

--
Cheers

Koz
81b61875e41eaa58887543635d556fca?d=identicon&s=25 Frederick Cheung (Guest)
on 2008-04-27 13:25
(Received via mailing list)
On 26 Apr 2008, at 23:27, Michael Koziarski wrote:

>
>> Sounds like a plan to me. I'm happy to knock up the (completely
>> trivial) patch.
>
> Sounds great, we should also add a deprecation warning to 2-0-stable,
> care to whip up both patches? ;)
>
http://rails.lighthouseapp.com/projects/8994-ruby-...

Fred
This topic is locked and can not be replied to.