Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ci skip] Move association class method notes #14211

Merged
merged 1 commit into from
Feb 26, 2014
Merged

Conversation

loganhasson
Copy link
Contributor

Make explanation of association class methods clearer by moving notes to beginning of each example section.

@arthurnn
Copy link
Member

@fxn should we also remove the word Note ?

@loganhasson
Copy link
Contributor Author

@arthurnn @fxn updated. You're right... Note everywhere seems superfluous.

@dmathieu
Copy link
Contributor

The parenthesis seem to be superfluous too.

@arthurnn
Copy link
Member

@loganhasson thanks, can you, also, squash your commits in one?

@loganhasson
Copy link
Contributor Author

@arthurnn yep, will do. Should I get rid of the parenthesis too?

@arthurnn
Copy link
Member

@loganhasson yep please.

@loganhasson
Copy link
Contributor Author

@arthurnn done and done.

@fxn
Copy link
Member

fxn commented Feb 26, 2014

Not convinced about the wording: collection is not being replaced by anything right? It is more of a documentation placeholder... something in that line would be better I believe.

@loganhasson
Copy link
Contributor Author

@fxn you're right...that wording is strange.

+collection+ is a placeholder for the symbol passed as the first argument, so <tt>has_many :clients</tt> would add among others <tt>clients.empty?</tt>.

slightly better?

@fxn
Copy link
Member

fxn commented Feb 26, 2014

👍

Make explanation of association class methods clearer by moving notes to beginning of each example section.
@loganhasson
Copy link
Contributor Author

@fxn @arthurnn fixed

@fxn
Copy link
Member

fxn commented Feb 26, 2014

This edit is good, those placeholders are not obvious for a newcomer and this remark is better there upfront. Thanks!

fxn added a commit that referenced this pull request Feb 26, 2014
[ci skip] Move association class method notes
@fxn fxn merged commit 3225898 into rails:master Feb 26, 2014
@arthurnn
Copy link
Member

thanks ❤️

@loganhasson
Copy link
Contributor Author

@fxn @arthurnn thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants