Skip to content

Conversation

@jameshfisher
Copy link
Contributor

No description provided.

@jameshfisher jameshfisher self-assigned this Nov 13, 2017
@jameshfisher jameshfisher requested a review from kn100 November 13, 2017 18:13
@kn100
Copy link
Contributor

kn100 commented Nov 13, 2017

LGTM!

Copy link
Contributor

@luismfonseca luismfonseca left a comment

Choose a reason for hiding this comment

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

LGTM.
But it could panic with a prettier message :o

@jameshfisher jameshfisher merged commit ff0a9fe into master Nov 14, 2017
@jameshfisher jameshfisher deleted the check-viper-error branch November 14, 2017 11:24
@jameshfisher
Copy link
Contributor Author

IMO panics should be big, explicit and ugly

@luismfonseca
Copy link
Contributor

luismfonseca commented Nov 14, 2017

But it's foreseeable that the configuration could be wrong.
We don't need to expose a stacktrace to the user as that is just not helpful.

fbenevides pushed a commit that referenced this pull request May 3, 2023
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.

4 participants