search

meiersi / blaze-builder

Efficient serialization of Haskell values to lazy bytestrings with a large average chunk size.
http://jaspervdj.be/blaze
Other
21 stars 22 forks source link

Unify with bytestring Builder #17

Open aristidb opened 10 years ago

aristidb commented 10 years ago

Are there plans to unify blaze-builder with bytestring builder?

alevy commented 10 years ago

+1

chrisdone commented 9 years ago

Is there a migration path to bytestring-builder?

meiersi commented 9 years ago

Yes, there is. @lpsmith has done most of the work to provide a compatibility release with his https://github.com/lpsmith/bytestring-builder library and the compatibility branch in this repo. It is up to me to finally release version 0.4 of blaze-builder that makes use of this good work.

aristidb commented 9 years ago

Sounds good!

Simon Meier notifications@github.com schrieb am Tue Jan 27 2015 at 19:43:00:

Yes, there is. @lpsmith https://github.com/lpsmith has done most of the work to provide a compatibility release with his https://github.com/lpsmith/bytestring-builder library and the compatibility branch in this repo. It is up to me to finally release version 0.4 of blaze-builder that makes use of this good work.

— Reply to this email directly or view it on GitHub https://github.com/meiersi/blaze-builder/issues/17#issuecomment-71703006 .

chrisdone commented 9 years ago

That sounds great, @meiersi. I look forward to it.

lpsmith commented 9 years ago

Ok, I just sent a pull request to Simon for the latest version of my compat work. it does pass the old test suite, but the test suite is not comprehensive and it could be tested better. IIRC, this buglet passed the suite, but was caught upon inspection of the output of some manual ad-hoc tests.

In any case, the existing work should be pretty close to correct. I am using this compatibility layer in a pretty minimal way in production, where I have a daemon that's using the new builder that also touches a postgres database via postgresql-simple, which has blaze-builder as part of it's public API. However, what it's doing via the compatibility layer is very simple so I wouldn't consider it that strong of evidence of correctness either. In particular, I'm pretty sure that buglet would also not have impacted that daemon.

I'm also of the opinion that, as long as the programmers put a reasonable and honest effort into making it correct, that releasing the software is an effective means of exposing any lingering problems.

snap depends on the old internal modules, which are no longer available in the compatibility layer. I looked at getting snap running on the new builder, but it always turned out to be a bit more effort than I was willing to put into it at that point in time.

lpsmith commented 9 years ago

So, things that should be done before release:

  1. merge the newer parts of the master branch. Some of the newer changes in master are no longer relevant, but some are, e.g. the .travis.yml file.
  2. Investigate the buglet I pointed out, and add a regression test if necessary.
  3. Code review... I don't think that an adequate code review needs to be extremely detailed, but the build parameters and in particular the dependency version constraints do need some work.
meiersi commented 9 years ago

Hi Leon,

thank you very much for this great work. I'll get to work on that and your pull request this Friday.

meiersi commented 9 years ago

I've looked at the code and I think there is one further issue that we should fix.

  1. Deprecate the library for general use and point people to the bytestring builder.
lpsmith commented 9 years ago

Ok, after a chat with Simon, we decided that I'm going to take over maintainership of blaze-builder. Also, I finally released blaze-builder-0.4.0.0, which is mostly just a wrapper around the new builder.

meiersi commented 9 years ago

Thanks a lot @lpsmith for taking over and handling this release!

2015-02-12 11:15 GMT+01:00 Leon P Smith notifications@github.com:

Ok, after a chat with Simon, we decided that I'm going to take over maintainership of blaze-builder. Also, I finally released blaze-builder-0.4.0.0, which is mostly just a wrapper around the new builder.

— Reply to this email directly or view it on GitHub https://github.com/meiersi/blaze-builder/issues/17#issuecomment-74047279 .

chrisdone commented 9 years ago

This sounds awesome.

So if I migrate my library to bytestring-builder, is there some BS.Builder -> Blaze.Builder function that users of my library can use? E.g. http-client expects a blaze-builder, and e.g. lucid would expose a bytestring-builder, so there'd need to be a (hopefully essentially no-op) conversion between the two types.

(Otherwise I can't reasonably upgrade to bytestring-builder without making my users unable to use my library with other libraries that expect blaze.)

lpsmith commented 9 years ago

@chrisdone Blaze.Builder is just a reexport of the BS.Builder type, so the function you are looking for is id. ;-)

chrisdone commented 9 years ago

@lpsmith Okay, so I should wait until packages using blaze-builder have upgraded to this version before I can start using bytestring-builder.

lpsmith commented 9 years ago

@chrisdone, if the packages you want to use only use the public interface, the only thing you would need to do is (possibly) bump the upper bounds. If they touch the internals, like snap does for instance, then yes, you'll have to wait.

lpsmith commented 9 years ago

Although, perhaps I should tweak the documentation a bit to clarify the issue that Blaze.Builder and BS.Builder are exactly the same type.

chrisdone commented 9 years ago

No problem, it seems that it'll be a straight-forward bump for package maintainers. This stackage issue gives a good indication of what needs to be updated.

lpsmith commented 9 years ago

Well, snap is a bit more involved, but it still shouldn't be a serious issue. I wrote up a more detailed blog post regarding this new release.