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

Custom Json Error messages for Argonaut and Circe #2316

Merged
merged 7 commits into from Jan 31, 2019

Conversation

@zarthross
Copy link
Member

@zarthross zarthross commented Dec 15, 2018

The direction I was taking was to allow overriding of the error messages in the traits .
I'm open to using implicits instead, just need suggestions.

  • Jawn Custom Parse Error Messages
  • Argonaut Custom Decode Errors
  • Circe Custom Decode Errors
Copy link
Member

@rossabaker rossabaker left a comment

There is already a withPrettyParams on the ArgonautInstances companion. I wonder if we should have with methods on the instances themselves, making them an abstract class along the lines of the backend builders. Then the customizations could be chained:

val instances = ArgonautInstances.withJawnParseExceptionMessage(pe).withPrettyParams(pp)
import instances._

It's ugly, but orphan instances are inherently ugly, and a newtype isn't all that convenient to use.

What do you think?

@zarthross zarthross force-pushed the Custom-Json-Errors branch from 6cb3ffd to 437fe3e Dec 22, 2018
@zarthross
Copy link
Member Author

@zarthross zarthross commented Dec 22, 2018

@rossabaker Switched to a builder pattern to see what it would look like. I can continue with that if I get the 👍 up.

@rossabaker
Copy link
Member

@rossabaker rossabaker commented Dec 25, 2018

This seems like it's on the right track to me.

@zarthross zarthross force-pushed the Custom-Json-Errors branch from 10efd72 to 50a3a22 Jan 26, 2019
@zarthross zarthross changed the title WIP: Custom Json Error messages Custom Json Error messages for Argonaut and Circe Jan 26, 2019
@zarthross
Copy link
Member Author

@zarthross zarthross commented Jan 26, 2019

@rossabaker Hey, mind looking this over again? I think I've got circe and argonaut done.

I would think if we want to do the same for the following it should probably be a separate PR.

  • boopickle
  • json4s-*
  • play-json

@zarthross
Copy link
Member Author

@zarthross zarthross commented Jan 26, 2019

Note: Last commit failed to build for what appears to be the same failure that's plaguing the master branch right now.

Copy link
Member

@rossabaker rossabaker left a comment

Agreed on the test failure. This looks good to me.

@ChristopherDavenport ChristopherDavenport merged commit e025074 into http4s:master Jan 31, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@zarthross zarthross deleted the Custom-Json-Errors branch Jul 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants