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

Add hoistServer to HasServer #804

Merged
merged 2 commits into from Sep 14, 2017
Merged

Conversation

phadej
Copy link
Contributor

@phadej phadej commented Sep 8, 2017

This is a refinement of an idea by @paf31.

I'll do documentation pass and add tests, if the idea is OK.

@phadej phadej requested review from alpmestan and jkarni Sep 8, 2017
@phadej phadej added this to the 0.12 milestone Sep 8, 2017
Copy link
Contributor

@alpmestan alpmestan left a comment

I quite like it. It's a bit of a shame that we'd have to define something identical for Client if we wanted to be able to operate on clients like this. Also, would we remove Enter altogether, with this? Maybe over the course of a few major releases, to give users some time to port their code. It might be nice to have a blog post for the first release that'll ship with this patch, to clearly and loudly document this breaking change.

@phadej
Copy link
Contributor Author

phadej commented Sep 8, 2017

I'd only deprecate enter, and keep it around for few major releases

@phadej
Copy link
Contributor Author

phadej commented Sep 14, 2017

@alpmestan, should I rewrite whole Enter section in the tutorial (i.e. only mention it existed?)

@alpmestan
Copy link
Contributor

alpmestan commented Sep 14, 2017

@phadej Well, we definitely want to explain the new stuffs. And maybe indeed add a little sentence or two that says "it replaces Enter, just do this to port your enter-based code over to hoistServer" ?

@phadej
Copy link
Contributor Author

phadej commented Sep 14, 2017

@alpmestan I'll merge this now and update tutorial in separate PR (as I guess there will be much more to review).

@phadej phadej merged commit 97aa7db into haskell-servant:master Sep 14, 2017
1 check passed
@phadej phadej deleted the hoist-server branch Sep 14, 2017
@alpmestan
Copy link
Contributor

alpmestan commented Sep 14, 2017

@phadej very good idea :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants