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

Simplify NetworkLayer #63

Merged
merged 2 commits into from
Mar 14, 2019
Merged

Simplify NetworkLayer #63

merged 2 commits into from
Mar 14, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Mar 14, 2019

Relates to #12

Overview

This carries on with the simplifications from #60.

  • it removes ReaderT in favour of passing the HttpBridge as a function argument.
  • it adds a newNetworkLayer function
  • it squashes the details from ServantError into HttpBridgeError

@rvl rvl self-assigned this Mar 14, 2019
@rvl rvl requested review from jonathanknowles and KtorZ March 14, 2019 04:20
@rvl rvl mentioned this pull request Mar 14, 2019
3 tasks
@KtorZ KtorZ force-pushed the rvl/12/review-network-layer branch from 2fbbd44 to 3dfd7e5 Compare March 14, 2019 09:33
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

LGTM. I've added a small change on the mapUntilError function, mostly modifying the type-signature to better reflect on what the function is doing. Since it discards the error and can never fail, it makes more sense to return a m [b] rather than a Except e m [b].

@KtorZ KtorZ changed the base branch from KtorZ/review-slotting-as-primitive to master March 14, 2019 09:35
rvl and others added 2 commits March 14, 2019 10:36
The docstring says it can never fail, so I've reflected that in the type signature too. We are actually discarding the
error here, so let's be honest about it :p
@KtorZ KtorZ force-pushed the rvl/12/review-network-layer branch from 3dfd7e5 to 1bfdbff Compare March 14, 2019 09:36
@KtorZ KtorZ merged commit 248369b into master Mar 14, 2019
@rvl rvl mentioned this pull request Mar 14, 2019
6 tasks
@KtorZ KtorZ deleted the rvl/12/review-network-layer branch March 22, 2019 15:04
@KtorZ KtorZ mentioned this pull request Mar 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
Development

Successfully merging this pull request may close these issues.

2 participants