search

clj-commons / manifold

A compatibility layer for event-driven abstractions
1.02k stars 106 forks source link

Allow function passed to `time/in` to return a deferred value #182

Closed sonntag closed 3 years ago

sonntag commented 4 years ago

In the current implementation of time/in, if the passed function returns a deferred value then after everything is done it results in a deferred wrapping a deferred. This changes it so the deferred returned by the passed function is connected through.

sonntag commented 3 years ago

@slipset any chance we can get this merged in?

slipset commented 3 years ago

Sorry, I dropped the ball on this. I would really appreciate it if @KingMob would have a look and approve.

KingMob commented 3 years ago

This looks good to me.

I'm trying to think of a counter-scenario where this breaks something, and while it's possible, getting a deferred out of a deferred isn't usually what we want, and pervasive use of unwrap amounts to the same thing.

One request: @sonntag can you update the docstring to make it clear that the return value of f will be unwrapped?

KingMob commented 3 years ago

@sonntag? I can merge once the docstring is updated. I think it would be nice to get this in before cutting a 0.2.0 release.

sonntag commented 3 years ago

@KingMob Just updated the docstring for 2 affected functions. I'm not the best at writing docstrings so up for feedback.

KingMob commented 3 years ago

I think "unwrapped value" is more clear than "deferred value", but I'll go ahead and change that myself.