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 missing constructors to ContextRoutes #7123

Merged
merged 3 commits into from May 24, 2023

Conversation

hamnis
Copy link
Contributor

@hamnis hamnis commented May 23, 2023

Added some useful constructors to ContextRoutes that existed on HttpRoutes.

@mergify mergify bot added series/0.23 PRs targeting 0.23.x module:core labels May 23, 2023
Added some useful constructors to ContextRoutes that existed on
HttpRoutes.
@hamnis hamnis force-pushed the contextroutes-constructors branch 2 times, most recently from 63fca36 to 1092c52 Compare May 23, 2023 18:21
@hamnis hamnis force-pushed the contextroutes-constructors branch from 1092c52 to 21d7a6a Compare May 23, 2023 18:26
@hamnis hamnis force-pushed the contextroutes-constructors branch from a759e95 to 4459251 Compare May 24, 2023 07:39
Copy link
Member

@danicheg danicheg left a comment

Choose a reason for hiding this comment

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

Thanks for iterating over this! Despite my minor comment, this looks good to me

*/
def local[T, F[_]: Monad](f: ContextRequest[F, T] => ContextRequest[F, T])(
fa: ContextRoutes[T, F]
): ContextRoutes[T, F] = Kleisli(req => Monad[OptionT[F, *]].unit >> fa.run(f(req)))
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps I'm missing something, but do we need here flatmapping over F[Unit]?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it is similar to Http.local – i.e. deferring without the Defer constraint per se.

Copy link
Member

@danicheg danicheg May 24, 2023

Choose a reason for hiding this comment

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

Yeah, thank you. I rather meant, is this a thing actually (in this particular use-case)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't know if the effect within the Kleisli fa is deferred, so this is just to make sure it is.

Copy link
Member

Choose a reason for hiding this comment

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

Is it all about awareness of whether the F has eager semantics of computation?

@hamnis hamnis force-pushed the contextroutes-constructors branch from 4459251 to 4f3e4e8 Compare May 24, 2023 11:58
Co-authored-by: Daniel Esik <e.danicheg@yandex.ru>
@hamnis hamnis force-pushed the contextroutes-constructors branch from 4f3e4e8 to 0a66f72 Compare May 24, 2023 12:20
@hamnis hamnis merged commit eb9c590 into http4s:series/0.23 May 24, 2023
16 checks passed
@hamnis hamnis deleted the contextroutes-constructors branch May 24, 2023 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:core series/0.23 PRs targeting 0.23.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants