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

Cleaning up ActiveModel::Dirty tests #8940

Merged
merged 1 commit into from
Jan 15, 2013
Merged

Cleaning up ActiveModel::Dirty tests #8940

merged 1 commit into from
Jan 15, 2013

Conversation

adomokos
Copy link
Contributor

  • Clarifying what the #changed method returns
  • Adding tests to describe what the #changed_attributes returns

@sikachu
Copy link
Member

sikachu commented Jan 14, 2013

👍 on the added test, but 👎 on the test naming convention. I don't think Rails does have a convention of 'prefix the test with method name'.

@adomokos
Copy link
Contributor Author

@sikachu: What do you think now?


@model.name = "John"
assert_equal({ "name" => "Paul" }, @model.changed_attributes)
end

Choose a reason for hiding this comment

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

Seems fine, I think you can move it to here. Thanks.

* Clarifying what the #changed method returns
* Adding tests to describe what the #changed_attributes returns
* Updating test name based on pull request comment
* Moving the test lower in the file per pull request comment
@adomokos
Copy link
Contributor Author

@carlosantoniodasilva: I moved it. Let me know if this works.

carlosantoniodasilva added a commit that referenced this pull request Jan 15, 2013
…butes

Cleaning up ActiveModel::Dirty tests
@carlosantoniodasilva carlosantoniodasilva merged commit 203f787 into rails:master Jan 15, 2013
@adomokos
Copy link
Contributor Author

Thanks!

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

3 participants