-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add save! to raise exception if things go wrong #19
base: master
Are you sure you want to change the base?
Conversation
lib/active_form/base.rb
Outdated
@@ -42,6 +42,10 @@ def save | |||
end | |||
end | |||
|
|||
def save! | |||
save or raise ActiveRecord::RecordInvalid.new(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK ||
syntax if prefered over or
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kirs is right in the syntax. But overall this is a no go for me because we're not targeting Active Record as the Rails integration point, but Active Model.
A record invalid or form invalid error using the Active Form namespace would be fine for me.
Kasper
Den 25/11/2014 kl. 22.17 skrev Kir Shatrov [email protected]:
In lib/active_form/base.rb:
@@ -42,6 +42,10 @@ def save
end
end
- def save!
AFAIK || syntax if prefered over orsave or raise ActiveRecord::RecordInvalid.new(self)
—
Reply to this email directly or view it on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're not targeting Active Record as the Rails integration point, but Active Model.
A record invalid or form invalid error using the Active Form namespace would be fine for me.
Good point, I totally agree.
I'll be happy to merge it right after #18 |
This reverts commit 6275c7b.
No description provided.