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

Introduce MSFAsync #336

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Conversation

turion
Copy link
Contributor

@turion turion commented Nov 3, 2022

Work in progress to clean up the module Data.MonadicStreamFunction.Async.

@ivanperez-keera would you in principle merge this? Then I would clean up, and document.

@turion
Copy link
Contributor Author

turion commented Nov 3, 2022

I got the basic insight from @AshleyYakeley in haskell/mtl#85 (comment). Since MSF (MaybeT m) a b == ListT (ReaderT a m) b, it should have a monad instance. This PR implements it, and gets the function concatS for free.

@ivanperez-keera
Copy link
Owner

A couple of comments:

  • Can you first create an issue to document what's happening?
  • Don't mix fixes to other things as part of the same PR (even if they are in the same module).
  • The new type should be by default hidden in the module. It's an implementation artifact.
  • How does the new implementation of concatS work? It looks pretty complicated to me. Can you document it and potentially introduce intermediate names (and types) for the subexpressions?

@ivanperez-keera
Copy link
Owner

BTW we also have a discussion tab were we can start the conversation. One things are clearer we can turn it into an issue. That can help separate long conversations (discussions) from more formal (and succinct) change proposals (issues).

@turion
Copy link
Contributor Author

turion commented Nov 8, 2022

Can you first create an issue to document what's happening?

This is #338

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