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.

Sign up for GitHub

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

Jump to bottom

Improve the acceptable error #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve the acceptable error #15

wants to merge 1 commit into from

Conversation

junhuif
Copy link
Member

@junhuif junhuif commented Jan 14, 2017

There is two type of validation error that returned by db.Create.
This change support detect multiple type of errors.

Close #13.

@junhuif
Improve the acceptable error
8ad54a4 
There is two type of validation error that returned by `db.Create`.
This change support detect multiple type of errors.

Close #13.
@junhuif junhuif self-assigned this Jan 14, 2017
@junhuif junhuif requested a review from bodhi January 14, 2017 08:13
bodhi
bodhi reviewed Jan 15, 2017
View reviewed changes
Copy link
Member

@bodhi bodhi left a comment

Choose a reason for hiding this comment

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

Would it be better to not panic in the handler, and let this callback do whatever it needs to do?

@junhuif
Copy link
Member Author

junhuif commented Jan 24, 2017

Would it be better to not panic in the handler, and let this callback do whatever it needs to do?

Yeah. It would be smarter. But in this way, this handler does nothing just let the user handle the error itself. And panic is kind of a conventional rule in the handlers in this package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bodhi bodhi bodhi left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@junhuif junhuif

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@junhuif @bodhi