Update "Active Record Associations" guide [ci skip] #31316
Conversation
|
(@rails-bot has picked a reviewer for you, use r? to override) |
|
Merely saying "Update" in the commit message doesn't cut it for me. Please elaborate what you've changed and why. |
bogdanvlviv
force-pushed
the
update-association_basics-guide
branch
from
a1fe3fc
to
6347fe8
Compare
What's wrong about this? |
bogdanvlviv
force-pushed
the
update-association_basics-guide
branch
from
6347fe8
to
8cc7c3c
Compare
|
@kaspth Thanks for the review. looks like I was misunderstanding the sentence |
guides/source/association_basics.md
Outdated
| The `collection.find` method finds objects within the collection. It uses the same syntax and options as `ActiveRecord::Base.find`. It also adds the additional condition that the object must be in the collection. | ||
| The `collection.find` method finds objects within the collection. It uses the same syntax and options as | ||
| [`ActiveRecord::Base.find`](http://api.rubyonrails.org/classes/ActiveRecord/FinderMethods.html#method-i-find). | ||
| It also adds the additional condition that the object must be in the collection. |
There was a problem hiding this comment.
The
collection.findmethod finds objects within the collection. [...] It also adds the additional condition that the object must be in the collection.
This seems totally redundant to me; I think you were right to remove it initially.
I think what it's trying to say is that collection.find behaves identically to ActiveRecord::Base.find, except that the query will be scoped to the collection. I think that's clear enough from the first two sentences, though.
WDYT @kaspth?
There was a problem hiding this comment.
Yes, the line is totally redundant and can be removed. @bogdanvlviv has expressed interest in upping his contributions, so I'm badgering him to explain his understanding and come up with better arguments for it.
In other words: the line shouldn't be removed because "it's wrong" (it ain't), but it should be removed because it's redundant. Bogdan, you know what to do 😊
There was a problem hiding this comment.
Apologies, I didn't recognise the Socratic method at work here. 😁
There was a problem hiding this comment.
@eugeneius pretty hidden, so no worries! 😊
@bogdanvlviv cool 👍
- Make all `ActiveRecord::Base.find` as link
- Remove redundant sentences
"It also adds the additional condition that the object must be in the collection."
bogdanvlviv
force-pushed
the
update-association_basics-guide
branch
from
8cc7c3c
to
ff214c3
Compare
Make all
ActiveRecord::Base.findas linkRemove "It also adds the additional condition that the object must be in the collection."sentence for
collection.wheresince it isn't correct statement.Remove the sentence for
collection.findfor consistency with others.Remove redundant sentences
"It also adds the additional condition that the object must be in the collection."