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

Export the yieldM on ConduitM. #270

Merged
merged 1 commit into from Aug 8, 2016

Conversation

Projects
None yet
2 participants
@tmcgilchrist
Contributor

tmcgilchrist commented Jun 30, 2016

Not sure the reasons why this isn't exported but I find it rather useful for code I want to keep pure and only mix into conduit at the last minute.

eg

deriveFeatures :: Monad m =>  Conduit Row (EitherT RowError m) OtherRow
deriveFeatures od = awaitForever (yieldM . hoistEither . deriveFeatures')

deriveFeatures' :: Row -> Either JoinOfferError OtherRow
deriveFeatures' = ....
@snoyberg

This comment has been minimized.

Owner

snoyberg commented Jun 30, 2016

I'm OK exporting it, but it will need:

  • A Haddock comment explaining when it's useful vs yield
  • A @since line in the comment
  • A mention in the ChangeLog

Could you make such changes to this PR?

@tmcgilchrist

This comment has been minimized.

Contributor

tmcgilchrist commented Jun 30, 2016

Sure, I can do that.

After speaking to people here at work, I'm less convinced that it's a useful thing to have in my case. Most of what I need can be achieved using mapM and filterM rather than needing yieldM directly.

@snoyberg

This comment has been minimized.

Owner

snoyberg commented Jun 30, 2016

FWIW, you could also write your example above as:

awaitForever (lift . yield <=< hoistEither . deriveFeatures')
@tmcgilchrist

This comment has been minimized.

Contributor

tmcgilchrist commented Jul 22, 2016

Finally got some time to add the docs, etc.
Is this right @snoyberg ?

@@ -1,3 +1,7 @@
## 1.2.6.7

This comment has been minimized.

@snoyberg

snoyberg Jul 22, 2016

Owner

This would be a minor version bump, not a patch, since it's not forwards-compatible (i.e., 1.2.7).

@@ -842,6 +842,9 @@ yield :: Monad m
yield o = yieldOr o (return ())
{-# INLINE yield #-}
-- | Send a monadic value to the next component to consume.
--
-- Since 1.2.6.7

This comment has been minimized.

@snoyberg

snoyberg Jul 22, 2016

Owner

Could you use the newer @since syntax? I know most of the other docs don't do that yet...

@@ -842,6 +842,9 @@ yield :: Monad m
yield o = yieldOr o (return ())
{-# INLINE yield #-}
-- | Send a monadic value to the next component to consume.

This comment has been minimized.

@snoyberg

snoyberg Jul 22, 2016

Owner

I think it would be worth including the word "downstream," and explaining why this is different from yield.

@snoyberg snoyberg merged commit 2a63a76 into snoyberg:master Aug 8, 2016

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