-
-
Notifications
You must be signed in to change notification settings - Fork 1
chore!: Use setup-ocaml@v3
#112
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
Conversation
Can you pin to the hashes like I did in grain-lang/binaryen.ml#201 ? |
Done as a separate question do we want to bump the node version we use to at least |
22 is fine since it's LTS now. I like it in the other PR. |
setup-ocaml@v3
and enable v5
buildssetup-ocaml@v3
package.json
Outdated
@@ -5,7 +5,7 @@ | |||
"author": "Blaine Bublitz <blaine@grain-lang.org>", | |||
"license": "Apache-2.0", | |||
"dependencies": { | |||
"ocaml": ">= 4.12.0 < 5.0.0", | |||
"ocaml": ">= 4.13.0 < 5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be updated to support 5.0.x releases:
"ocaml": ">= 4.13.0 < 5.0.0", | |
"ocaml": ">= 4.13.0 < 5.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't my other pr for supporting ocaml 5 do that?
feat: Support OCaml 5 chore!: Drop support for OCaml 4.12
bf96a4b
to
bbd8fe7
Compare
I fixed up the versions. As long as CI passes this is good to go. |
Woo! |
This pr switches from
setup-ocaml@v2
tosetup-ocaml@v3
.I was trying to keep the
CC
andCCX
changes out of the dune file but I couldn't find a way to override them in the opam file that was platform dependent, and I didn't see a real downside to doing it in the dune file.Note:
4.12.1
assetup-ocaml@v2
doesn't support it5.3.0
in its place so we can hopefully begin moving to ocamlv5
Closes: #96