xguerin / bitstring

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

use ppxlib #22

Closed pveber closed 3 years ago

pveber commented 3 years ago

This is a proposal for basing the ppx extension on ppxlib, as ppx_tools_versioned is going to be deprecated. This new version passes all tests performed by a dune runtest invocation.

xguerin commented 3 years ago

Thanks for the PR.

The opam files are automaticallty generated by dune. Also, I split bitstring into 2 packages: bitstring and ppx_bitstring. Could you add a commit with the following patch to your PR:

diff --git a/bitstring.opam b/bitstring.opam
index edff042..4dbc072 100644
--- a/bitstring.opam
+++ b/bitstring.opam
@@ -16,7 +16,6 @@ depends: [
   "dune" {>= "2.5"}
   "ocaml" {>= "4.02.3"}
   "stdlib-shims" {>= "0.1.0"}
-  "ppxlib" {>= "0.16.0"}
 ]
 build: [
   ["dune" "subst"] {pinned}
diff --git a/dune-project b/dune-project
index 094f53f..4aeb5ac 100644
--- a/dune-project
+++ b/dune-project
@@ -36,6 +36,6 @@
  (depends
   (bitstring (>= 3.2.0))
   (ocaml (or (and :with-test (>= 4.08.0)) (and (= :with-test false) (>= 4.02.3))))
-  (ppx_tools_versioned (and :build (>= 5.0.0)))
+  (ppxlib (and :build (>= 0.10.0)))
   (ocaml-migrate-parsetree (and :build (>= 1.0.5)))
   (ounit :with-test)))
diff --git a/ppx_bitstring.opam b/ppx_bitstring.opam
index 3fd9caf..d959e49 100644
--- a/ppx_bitstring.opam
+++ b/ppx_bitstring.opam
@@ -16,7 +16,7 @@ depends: [
   "dune" {>= "2.5"}
   "bitstring" {>= "3.2.0"}
   "ocaml" {with-test & >= "4.08.0" | with-test = "false" & >= "4.02.3"}
-  "ppx_tools_versioned" {build & >= "5.0.0"}
+  "ppxlib" {build & >= "0.10.0"}
   "ocaml-migrate-parsetree" {build & >= "1.0.5"}
   "ounit" {with-test}
 ]

That should do the trick.

pveber commented 3 years ago

Thanks Xavier! Is the specification of version 0.10.0 for ppxlib intended? I only tested with 0.16.0, and I'm pretty sure the patch wouldn't compile with earlier versions.

xguerin commented 3 years ago

Ah no my bad, that was a typo. I'll let you use 0.16.0 instead 👍

pveber commented 3 years ago

Done!

xguerin commented 3 years ago

Thanks!

One more thing. It looks like ppxlib is only available for ocaml >= 4.04.1. Would you mind:

  1. Adjusting the ocaml constraint in dune-project for both bitstring and ppx_bistring
  2. Removing 4.02 and 4.03 in the travis matrix in .travis.yml

Since most PPX rewriter are being ported to ppxlib, most of them will drop support for those versions as well.

pveber commented 3 years ago

Done!