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

Remove an unused dependency to bytes #969

Merged
merged 2 commits into from
Oct 27, 2022

Conversation

kit-ty-kate
Copy link
Member

lwt hasn't been supporting OCaml < 4.02 for quite a long time

@raphael-proust
Copy link
Collaborator

Thanks!

I think the change should also include:

diff --git a/docs/Makefile b/docs/Makefile
index 5485ae43d..6fcaaa67c 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -2,7 +2,7 @@ BLD=../_build/default/src
 SRC=../src
 
 PKGS=\
-	-package bytes -package result \
+	-package result \
 	-package bigarray -package unix \
 	-package ocaml-migrate-parsetree -package ppx_tools_versioned \
 	-package react \

do you concur? I can take care of adding this and the CHANGES entry to the PR before merging.

@kit-ty-kate
Copy link
Member Author

Yes indeed, though in this case ocamlfind is going to install a bytes package so it shouldn't matter. In the dune file it matters because bytes has stopped being a builtin with OCaml 5.0 (it only works because it relies on ocamlfind being installed via ocplib-endian)

Also I suspect this docs Makefile hasn't been run for a while (ppx_tools_versioned hasn't been updated since OCaml 4.11 i believe)

@raphael-proust
Copy link
Collaborator

The docs makefile was run not toooooo long ago (I made a special 4.13 switch, pinned a dedicated wikidoc package, and had to set up one or two more things). But I intend to remove it anyway in not too long.

I'll merge this PR after adding a CHANGES entry.

@raphael-proust raphael-proust merged commit fb47b31 into ocsigen:master Oct 27, 2022
@timothyklim timothyklim mentioned this pull request Dec 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants