-
Notifications
You must be signed in to change notification settings - Fork 85
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
Replace blaze-builder with bsb-http-chunked #109
base: master
Are you sure you want to change the base?
Conversation
import System.IO.Streams (InputStream, OutputStream) | ||
import qualified System.IO.Streams as Streams | ||
import System.IO.Streams.Attoparsec (parseFromStream) | ||
import Data.ByteString.Builder.HTTP.Chunked (chunkedTransferEncoding, chunkedTransferTerminator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change.
The Travis failure seems to be due to |
Yeah, I already opened this PR. basvandijk/threads#15 @gregorycollins Thoughts? |
@sjakobi Hmmm, when testing with GHC 8.4.1 |
Oof! For http-streams, I have already opened aesiniath/http-streams#108. I'll wait for any feedback there until (maybe) I do more work. http-common looks easy to fix but as with http-streams it's probably better to use a faster strict bytestring builder. I have opened aesiniath/http-common#14 |
Thanks for doing this work BTW. I was recently thinking that we should probably make the switch from blaze-builder to bytestring. Any chance you can modify |
Can you explain why that's important to you? I use bytestring-builder only to stay compatible with GHC < 7.8. I can hide the dependency again for recent GHCs like this if you prefer. |
Well, in general I like to minimize dependencies. I was thinking it wasn't buying us enough, but if it's buying us compatibility with < 7.8, then I think you're right, we should keep it for awhile longer. With snap we have a strong commitment to backwards compatibility. I just ditched 7.4 compatibility for |
Aren't you going to adjust your lower bound on base then? |
Oh, good point. I had removed the Travis test, but not bumped the lower bound. Just did that. |
@sjakobi It looks like your merge has an error in the cabal file. (The master branch is now building with GHC 8.4 btw.) |
I shouldn't have used the GitHub interface to fix the merge conflict… :) BTW, now that |
I have extracted
Blaze.ByteString.Builder.HTTP
into a new package.