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

BooPickle and Circe Decode Instances: more Cats #2564

Merged
merged 1 commit into from May 15, 2019

Conversation

@diesalbla
Copy link
Contributor

commented May 10, 2019

Both the BooPickle and the Circe decoder instances call to collectBinary and then applied an analysis to extract either the decoded result or a failure. This analysis phase was written as a function of Chunk to a DecodeResult, flatMapped to the result of collectBinary.

This PR rewrites it as a pure function of Chunk[Byte] to Either, which is applied to the result of collectBinary using the EitherT.subflatMap function.

@diesalbla diesalbla force-pushed the diesalbla:circe_decoder branch 3 times, most recently from 91bce44 to 4c8fa4f May 11, 2019

@diesalbla diesalbla changed the title BooPickle and Circe Decode Instances. BooPickle and Circe Decode Instances: more Cats May 11, 2019

@diesalbla diesalbla force-pushed the diesalbla:circe_decoder branch from 4c8fa4f to 99066f8 May 11, 2019

@rossabaker
Copy link
Member

left a comment

I like most of the change. One style quibble with the import.

@diesalbla diesalbla force-pushed the diesalbla:circe_decoder branch 2 times, most recently from 50b83d4 to daad826 May 12, 2019

@diesalbla

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

It turns out that the Try.toEither instance method is not available in version 2.11 of the library, so we can keep that as a match statement and have no need of the Either syntax there.

@diesalbla diesalbla force-pushed the diesalbla:circe_decoder branch from daad826 to e889243 May 14, 2019

BooPickle and Circe Decode Instances: redo.
In both the `BooPickle` and the `Circe` decoder instances, a call
to `collectBinary` is followed by an analysis to extract from the
chunk the result.

This analysis phase was written as a function of Chunk to a
DecodeResult, flatMapped to the `collectBinary`.
This PR rewrites them as pure functions of `Either` to `Either`,
which are applied to the result of `collectBinary` using
the `EitherT.subflatMap` function.

Import is gone

@rossabaker

This comment has been minimized.

Copy link
Member

commented May 15, 2019

Just needs a test:scalafmt, and the build should pass, and then 👍 from me.

@diesalbla diesalbla force-pushed the diesalbla:circe_decoder branch from e889243 to 43e0449 May 15, 2019

@rossabaker
Copy link
Member

left a comment

👍 on green

@rossabaker rossabaker merged commit e954f5d into http4s:master May 15, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@diesalbla diesalbla deleted the diesalbla:circe_decoder branch May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.