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

Provide an example of a Dune file in OCaml syntax #294

Closed
aantron opened this issue Mar 23, 2020 · 3 comments
Closed

Provide an example of a Dune file in OCaml syntax #294

aantron opened this issue Mar 23, 2020 · 3 comments
Labels
Milestone

Comments

@aantron
Copy link
Owner

aantron commented Mar 23, 2020

...which works around Dune not supporting a development-only PPX, ocaml/dune#57.

@aantron
Copy link
Owner Author

aantron commented Mar 23, 2020

@nobrowser, @anuragsoni, @Risto-Stevcev: I just changed the README instructions for Dune; they now recommend using Dune's OCaml syntax:

(* -*- tuareg -*- *)

let preprocess =
  match Sys.getenv "BISECT_ENABLE" with
  | "yes" -> "(preprocess (pps bisect_ppx))"
  | _ | exception _ -> ""

let () = Jbuild_plugin.V1.send @@ {|

(library
 (public_name my_lib)
 |} ^ preprocess ^ {|)

|}

I changed the starter repo accordingly, as a minimal example, and there is also a tidy patch showing the difference to dune files.

Doing it this way seems to work pretty well:

  • There is no need to edit for release.
  • No need for cleaning when switching between coverage and non-coverage builds. Dune seems to know which files need updating.

We've already been using this in odoc for a while (I had forgotten). The odoc version is slightly less tidy :)

I also just switched to Lwt to doing this (back, actually, we did this also in Lwt at some point in 2017 or so): ocsigen/lwt@7baf8b9

aantron added a commit that referenced this issue Mar 23, 2020
@Risto-Stevcev
Copy link

@aantron I figured out that since I'm writing a cross-compat library for bs + native, I don't actually need to have it run for native since I'm ensuring code coverage on the bs side. I just add it as a :with-test dependency and have it generate on the bs side instead. It might be worth noting that workflow in the docs because bucklescript folks won't be able to upgrade to the latest dune (stuck on 2.2.0 for bs) and would have to use the tuareg mode

@aantron
Copy link
Owner Author

aantron commented Mar 23, 2020

I'll keep this in mind for the future, for when there is a version of Dune that can offer a better Bisect integration on the native side.

anuragsoni added a commit to anuragsoni/routes that referenced this issue Mar 24, 2020
anuragsoni added a commit to anuragsoni/routes that referenced this issue Mar 24, 2020
anuragsoni added a commit to anuragsoni/routes that referenced this issue Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants