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

Using http-semantics #114

Merged
merged 14 commits into from
Apr 20, 2024
Merged

Using http-semantics #114

merged 14 commits into from
Apr 20, 2024

Conversation

kazu-yamamoto
Copy link
Owner

This implements #113.
Backward compatibility will lost for some functions of HPACK but I guess it's a minor issue.
warp can be built with this branch without modification.

@edsko Could you try to use this branch? If this breaks your projects, I should provide more deprecated functions or types.

@edsko
Copy link
Collaborator

edsko commented Apr 20, 2024

@kazu-yamamoto I had to make a few small adjustments, but only minor, and tests pass just fine.

Two of the changes I had to make were because there two places where I could have imported from .Client or .Server previously, but instead imported from .Internal because the things I needed were not client or server specific, and I did not want to make an arbitrary choice). So that's on me.

One small comment is that There is a downside to re-exporting entire modules (as in re-exporting Semantics.Server from HTTP2.Server): the haddocks for HTTP2.Server will just show that the other module is re-exported, but not what is in that module (until you click on it). This makes browsing the documentation a little more cumbersome. But it's very minor.

@kazu-yamamoto kazu-yamamoto merged commit 3bbcad5 into main Apr 20, 2024
10 checks passed
@kazu-yamamoto kazu-yamamoto deleted the semantics branch April 20, 2024 23:48
@kazu-yamamoto
Copy link
Owner Author

Thank you for your review.
Merged.
I agree that the haddock behavior is a little bit inconvenient.

edsko added a commit to edsko/http2 that referenced this pull request Jul 19, 2024
In kazu-yamamoto#92 we added an exception handler
that was meant to catch _all_ exceptions (sync and async). This got changed in
kazu-yamamoto#114 (specifically,
kazu-yamamoto@52a9619):
when we moved from `Control.Exception` to `UnliftIO.Exception`, we got a
different behaviour for `catch` and friends (see
well-typed/grapesy#193 (comment)) for a
full list. This commit fixes some unintended consequences of this change.
edsko added a commit to edsko/http2 that referenced this pull request Jul 23, 2024
In kazu-yamamoto#92 we added an exception handler
that was meant to catch _all_ exceptions (sync and async). This got changed in
kazu-yamamoto#114 (specifically,
kazu-yamamoto@52a9619):
when we moved from `Control.Exception` to `UnliftIO.Exception`, we got a
different behaviour for `catch` and friends (see
well-typed/grapesy#193 (comment)) for a
full list. This commit fixes some unintended consequences of this change.
edsko added a commit to edsko/http2 that referenced this pull request Jul 24, 2024
In kazu-yamamoto#92 we added an exception handler
that was meant to catch _all_ exceptions (sync and async). This got changed in
kazu-yamamoto#114 (specifically,
kazu-yamamoto@52a9619):
when we moved from `Control.Exception` to `UnliftIO.Exception`, we got a
different behaviour for `catch` and friends (see
well-typed/grapesy#193 (comment)) for a
full list. This commit fixes some unintended consequences of this change.
edsko added a commit to edsko/http2 that referenced this pull request Jul 24, 2024
In kazu-yamamoto#92 we added an exception handler
that was meant to catch _all_ exceptions (sync and async). This got changed in
kazu-yamamoto#114 (specifically,
kazu-yamamoto@52a9619):
when we moved from `Control.Exception` to `UnliftIO.Exception`, we got a
different behaviour for `catch` and friends (see
well-typed/grapesy#193 (comment)) for a
full list. This commit fixes some unintended consequences of this change.
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