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

Made 'withRunInIO' fully polymorphic #12

Merged
merged 3 commits into from Jan 2, 2018
Merged

Conversation

effectfully
Copy link
Collaborator

withRunInIO was

withRunInIO :: MonadUnliftIO m => ((m a -> IO a) -> IO b) -> m b

and now it's

withRunInIO :: MonadUnliftIO m => ((forall a. m a -> IO a) -> IO b) -> m b

I also replaced withUnliftIO by withRunInIO everywhere in the codebase. Docs are updated accordingly, but please verify. I think it should be mentioned somewhere in the library that withUnliftIO now has quite a little number of uses (basically, if you want to put a UnliftIO m into some data type) and withRunInIO is a preferable function.

@snoyberg
Copy link
Member

snoyberg commented Jan 1, 2018

I think it should be mentioned somewhere in the library that withUnliftIO now has quite a little number of uses

I'm on board with that, do you want to include that note in the Haddocks for withUnliftIO itself?

Copy link
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

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

Can you add a comment to the ChangeLog about the change in type signature, and update the version number to 0.1.1.0? Also, I believe we'll now need a lower bound on unliftio-core in unliftio.

@effectfully
Copy link
Collaborator Author

effectfully commented Jan 1, 2018

Done.

Personally, I think building the library around withRunInIO would make it easier to comprehend and there also won't be any hacky newtypes lying around which I think is a benefit. askUnliftIO can of course be implemented in terms of withRunInIO. What do you think?

In any case it's a great library: simple and useful. I'm currently in the pass-environments-explicitly camp, but I might reconsider this position in future, because now there is a sensible alternative. Thanks for this.

@snoyberg
Copy link
Member

snoyberg commented Jan 2, 2018 via email

@effectfully
Copy link
Collaborator Author

This discussion is irrelevant here, so I opened an issue.

Please say if there is something else that should be fixed in the pull request.

@snoyberg snoyberg merged commit 61ef439 into fpco:master Jan 2, 2018
@snoyberg
Copy link
Member

snoyberg commented Jan 2, 2018

Thanks!

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.

None yet

2 participants