search

xguerin / bitstring

OCaml Bitstring - bitstring matching for OCaml
GNU General Public License v2.0
67 stars 8 forks source link

Why not split ppx into own package? #11

Closed rgrinberg closed 4 years ago

rgrinberg commented 6 years ago

First of all, congratulations for getting everything under 1 repo and working with jbuilder. But I'm curious, why did you decide to fold the ppx package into bitstring? Ppx usually brings in extra dependencies and there are many use cases where you only want to use the package runtime. It's also just as easy to have 2 opam packages out of 1 repo as it is to have one.

xguerin commented 6 years ago

why did you decide to fold the ppx package into bitstring?

For multiple reasons:

I must admit I did not think about distributing 2 opam packages out of 1 repo. That being said, I do not quite see the value of bitstring without the syntax extension, especially since in this case the extra dependencies (ppx_tools_versioned and OMP) are rather light.

Nevertheless, I'll keep your suggestion open for people to comment on. If it becomes the obvious way to go I'll be glad to oblige.

rgrinberg commented 6 years ago

Xavier Guérin notifications@github.com writes:

  • It makes my life easier: 1 version to manage, 1 repo, 1 set of unit tests, 1 opam release.

Definitely not :). I'm no mad man and do not suggest to increase the maintenance of already over worked maintainers.

I must admit I did not think about distributing 2 opam packages out of 1 repo. That being said, I do not quite see the value of bitstring without the syntax extension, especially since in this case the extra dependencies (ppx_tools_versioned and OMP) are rather light.

Yeah, there's probably no value for direct uses. But such things to come up. For example, if I'd like to use the ppx and then publish my package with the generated code. It's helpful in such a situation to depend only on the runtime. This isn't a theoretical benefit either, it's quite easy to do if you use ppx_driver as your driver.

Other package managers (esy) rely on build time components being in a separate package to make cross compilation work.

Another small benefits is that it lets downstream users be more precise about their opam deps. For example, I notice that you've listed omp as a {build} dependency in the opam file. But that isn't quite correct as you do need to make sure omp is installed to run the ppx (I'll make a PR for that). But users of bitstring would be able to specify a normal dependency on bitstring and a {build} dependency on ppx_bitstring which is correct.

Nevertheless, I'll keep your suggestion open for people to comment on. If it becomes obvious way to go I'll be glad to oblige.

Thank you! And again a big thanks for taking over maintenance of this important package.

xguerin commented 4 years ago

Revisiting old issues ... :) I can update the dune project and provide two opam packages, one for the library and one for the ppx. I believe that would address your comment.