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

Fixes #7: Implement JsonCodec #28

Merged
merged 15 commits into from
Mar 18, 2021
Merged

Fixes #7: Implement JsonCodec #28

merged 15 commits into from
Mar 18, 2021

Conversation

thinkharderdev
Copy link
Contributor

Fixes #7

Still need to work out some of the tests but the codec implementation is done so wanted to get feedback in case I am way off base in the implementation.

@thinkharderdev
Copy link
Contributor Author

I think this is done. Need to get wait for a new version of zio-json to get cut with zio/zio-json#202 included so we can update dependency here.

Also, creating a real Gen[Random, ZoneId] seems to cause catastrophic performance issues any which way I try and do it so I have it defined using Gen.const(ZoneId.systemDefault()) for now.

@thinkharderdev thinkharderdev mentioned this pull request Mar 16, 2021
Merged
@thinkharderdev
Copy link
Contributor Author

Waiting on zio/zio-json#213 to get the JDK8 tests to pass. Also blocked on JDK8 Duration handling due to this bug: https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8054978

@jdegoes
Copy link
Member

jdegoes commented Mar 16, 2021

@thinkharderdev I merged your ZIO JSON pull request (thank you!). Can you work around the JDK8 bug?

@thinkharderdev
Copy link
Contributor Author

@thinkharderdev I merged your ZIO JSON pull request (thank you!). Can you work around the JDK8 bug?

Depends. I can "work around" it by using a Gen that doesn't produce durations which trigger it, but to really fix it we would need to re-implement duration parsing in zio-json.

@jdegoes
Copy link
Member

jdegoes commented Mar 16, 2021

@thinkharderdev I have logged the bug here so we can keep track of it and work around it in ZIO JSON, and then possibly, until that's done, we could just avoid generating such Duration by using the suchThat / filter operator on Gen. What do you think?

@jdegoes jdegoes mentioned this pull request Mar 16, 2021
Closed
@thinkharderdev
Copy link
Contributor Author

Yep. Works for me.

@thinkharderdev thinkharderdev marked this pull request as ready for review March 16, 2021 17:19
@jdegoes jdegoes merged commit e58a401 into zio:main Mar 18, 2021
@jdegoes
Copy link
Member

jdegoes commented Mar 18, 2021

Thank you for your work on this!

landlockedsurfer pushed a commit to landlockedsurfer/zio-schema that referenced this pull request May 28, 2022
@thinkharderdev @brbrown25 @landlockedsurfer
Fixes zio#7: Implement JsonCodec (zio#28)
5169e86 
* Initial commit of wip from https://github.com/zio/zio-web/pull/90. zio#7
>
Co-authored-by: Brandon Brown <brandon@bbrownsound.com>
Co-authored-by: Jason Pickens <jasonpickensnz@gmail.com>

* WIP trying to get build working

* wip

* Fixes zio#7: Implement JSON Codec

* Formatting

* More unit tests

* unit tests

* Add non-streaming methods to JsonCodec

* More test fixes

* Transform failure tests

* Update to published version of zio-json

* Scala 2.12 support

* Work around jdk 8 duration parsing bug

Co-authored-by: Brandon Brown <brandon@bbrownsound.com>
Co-authored-by: thinkharder <thinkharderdev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdegoes jdegoes jdegoes approved these changes

@mijicd mijicd Awaiting requested review from mijicd

@softinio softinio Awaiting requested review from softinio

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement JsonCodec using ZIO JSON
3 participants
@thinkharderdev @jdegoes @brbrown25