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] Improved Active Model Basics #42479

Merged

Conversation

AdityaBhutani
Copy link
Contributor

Summary

Improved Sentence by making it concise.

Fixed Grammatical Errors.

@rails-bot rails-bot bot added the docs label Jun 14, 2021
@@ -185,7 +185,7 @@ irb> person.changes
=> {"first_name"=>[nil, "First Name"]}
```

#### Attribute based accessor methods
#### Attribute-based accessor methods
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create a separate PR just fixing all "x based" constructs where it should be "x-based.
For example, I think the following should probably be "string-based" instead.

# # Cookie values are String based. Other data types need to be serialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p8 New x-based PR is here, 42485
Have updated this PR as well, or are we dumping this entire PR ?

@@ -65,7 +65,7 @@ irb> person.age_highest?
### Callbacks

`ActiveModel::Callbacks` gives Active Record style callbacks. This provides an
ability to define callbacks which run at appropriate times.
ability to define callbacks that run at appropriate times.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I prefer that here instead of which, which seems still allowed according to Merriam-Webster:

if your clause is bracketed by commas (“the article on grammar, which I started while eating lunch, seemed to never end”) it is likely a nonrestrictive clause, and you can give it a which. If it is not surrounded by commas, then it is most likely a restrictive clause, and you can choose to give it a that or a which. If anyone questions your decision, you can say that you are following the advice of the Fowlers, and are making a decision based on custom, euphony, and convenience.

https://www.merriam-webster.com/words-at-play/when-to-use-that-and-which

This change is arbitrary and someone else might just as well make the case to change it back to which.

@@ -323,7 +323,7 @@ objects.
### Serialization

`ActiveModel::Serialization` provides basic serialization for your object.
You need to declare an attributes Hash which contains the attributes you want to
You need to declare an attributes Hash that contains the attributes you want to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too arbitrary/cosmetic.

@@ -500,7 +500,7 @@ a `password` accessor with certain validations on it by default.

`ActiveModel::SecurePassword` depends on [`bcrypt`](https://github.com/codahale/bcrypt-ruby 'BCrypt'),
so include this gem in your `Gemfile` to use `ActiveModel::SecurePassword` correctly.
In order to make this work, the model must have an accessor named `XXX_digest`.
To make this work, the model must have an accessor named `XXX_digest`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is arbitrary/cosmetic.

@@ -485,8 +485,8 @@ Finished in 0.024899s, 240.9735 runs/s, 1204.8677 assertions/s.
6 runs, 30 assertions, 0 failures, 0 errors, 0 skips
```

An object is not required to implement all APIs in order to work with
Action Pack. This module only intends to provide guidance in case you want all
An object is not required to implement all APIs to work with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too arbitrary/cosmetic.

@zzak
Copy link
Member

zzak commented Jun 16, 2021

@AdityaBhutani Could you please squash your commits?

@AdityaBhutani AdityaBhutani force-pushed the fix-and-improve-guide-active-model-basics branch from 75d5cf4 to 2a3bf32 Compare June 16, 2021 13:04
@zzak zzak merged commit db9209d into rails:main Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants