forked from m-Peter/activeform
-
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
Open
leomayleomay
wants to merge
34
commits into
railsgsoc:master
Choose a base branch
from
fluxfederation:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
732f9c6
Add save! to raise exception if things go wrong
2739020
revert whitespace change for easier merging
willbryant 36c6653
merged rails/master
willbryant f17dd6b
Remove Gemfile.lock
209dfb3
Fix deprecation warnings
ccdfa22
Fix method signature for SimpleForm
a102adc
Fix attributes order in SimpleForm forms
939862e
Update README.md
faac7ec
Update License file
e4777dd
Update .gitignore
e893090
Fix deprecation warnings
1590bf4
Oopss.. typo
7a9dfb5
Remove empty lines
f39ef86
Setup tests with Appraisals
a7bb4c0
Fix warning
3238154
Add code coverage
72d9cb0
Update README.md
56542ca
Update README.md
2acfb03
Update README.md
30fe7b4
Fix tests with Rails 5 (part1)
6275c7b
Fix warning
b6f4535
Fix tests for Rails5
da36952
Revert "Fix warning"
b69e05a
Reduce Rails versions range in gemspec
6ec2f5f
Becareful of module namespace
612d09d
Add comment
c1cf72e
ActionForm should not depend on SimpleForm (only for tests)
c4382c0
Fix CodeClimate error
be5c214
Test with latest Ruby version
1ce4cdd
Fix CodeClimate reporter
n-rodriguez 52afb96
Test with latest Ruby version
n-rodriguez b6376ac
Test with latest Ruby versions
n-rodriguez 1860a35
Test with maintained versions of Rails
3fd79a7
Merge pull request #1 from jbox-web/master
willbryant File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 overor
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
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.
Good point, I totally agree.