This project is archived and is in readonly mode.
Add magic encoding comment to generated files
-
Will Bryant
+1 This inconsistency bugs me too. This patch works in my project and passes tests.
-
dvdplm
+1
Let's get this one in!
-
Manfred Stienstra
Hey guys, please don't just comment with just +1, it doesn't really help, it only annoys active developers who get assigned to the patch and they stop watching the tickets.
From the contributers guide:
If your comment simply says +1, then odds are other reviewers aren't going to take it too seriously. Show that you took the time to review the patch. Once three people have approved it, add the verified tag. This will bring it to the attention of a committer who'll then review the changes looking for the same kinds of things.
-
Neeraj Singh
I like the idea and the patch seems to work.
However I would like the patch to be extended to cover cases like this.
User.first(:limit => 3)
User.find(:first, :limit => 3)
If consistency is the name of the game then covering above two cases will make it more consistent.
I come across a lot of cases where the initial code was User.find(:first, :conditions => 'featured is not null') . And then later I need to show 3 users who are features. I would love to do User.find(:first, :conditions => 'featured is not null', :limit => 3)
-
dvdplm
@manfred: not sure how to "prove" that to you.
So, once again, with feeling: +1 because the patch works. Tests pass. The change is small, but brings value. It has docs. It makes sense. It allows me to do:
Email.last Email.last(10)
Email.first(:conditions => {:name => 'Beef'}) Email.first(3, :conditions => {:name => 'Beef'})
Don't know how to tell it. Can you have a look please?
-
Alex Chee
All the tests are passing on my projects and rails clone. I like this patch because it acts exactly how you would use Array's last and first.
-
Will Bryant
- Tag changed from activerecord, finder to activerecord, finder, verified
-
mikong
-1
This is a nice addition, but I wouldn't use this most of the time I use first or last, so I don't like to have the extra checking in there. And the current one doesn't read so bad:
Email.find(:all, :limit => 3)That's for first(3), but what about last? I think adding an explicit :order option is better. A lot has already added their plus ones but I just thought I should throw this thought in.
-
dstamat (at elctech)
@Raj: Good points, but I think that your examples don't necessarily read as well. For example:
User.first(:limit => 3)I read it as "Give me the first User, but limit to 3 results", which doesn't necessarily make sense.@mikong: Understand your point, but this simply adds a missing piece of the Array class to these finders. If Array can do this, I want ActiveRecord to be consistent:
[1, 2, 3].first(2) => [1, 2] [1, 2, 3].last(2) => [2, 3]
You could still add in custom ordering if you wanted to as well:
User.first(3, :order => "name desc") -
dstamat (at elctech)
Hmm... my 'code' blocks didn't render. Let me retry with the "at" symbol tag:
@Raj: Good points, but I think that your examples don't necessarily read as well. For example:
User.first(:limit => 3)I read it as "Give me the first User, but limit to 3 results", which doesn't necessarily make sense.
@mikong: Understand your point, but this simply adds a missing piece of the Array class to these finders. If Array can do this, I want ActiveRecord to be consistent:
[1, 2, 3].first(2) => [1, 2] [1, 2, 3].last(2) => [2, 3]You could still add in custom ordering if you wanted to as well:
User.first(3, :order => "name desc") -
Rohit Arondekar
- State changed from new to stale
- Importance changed from to
Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.
-
Jeff Kreeftmeijer
Automatic cleanup of spam.
