-
Notifications
You must be signed in to change notification settings - Fork 14
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
Adds integers_stubs_js & modifies js_of_ocaml. #23
Conversation
Update - ocsigen/js_of_ocaml#1296 got merged so I have added a commit to restore |
build: ["dune" "build" "-p" name "-j" jobs] | ||
dev-repo: "git+https://github.com/o1-labs/integers_stubs_js.git" | ||
url { | ||
src: "https://github.com/o1-labs/integers_stubs_js/archive/1.0.tar.gz" |
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 is the same as the upstream package, isn't it?
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.
Yes
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.
It wants to point to your branch, though - https://github.com/novemberkilo/integers_stubs_js/archive/master.tar.gz
(no checksum needed)
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.
The mainline repo should build with the updated js_of_ocaml branch so we shouldn't need my fork here?
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.
I'm just fixing a test related issue on my branch
Since the original package in opam-repository works and the tests aren’t ran I’ll close this. Thanks a lot for ocsigen/js_of_ocaml#1296 |
Owing to its revdeps, I investigated
integers_stubs_js
to see if I could get it building and installing with OCaml5. This is its failing build on ocaml-health-check.Apparently the issue was with its dependency -
js_of_ocaml-compiler
and following a helpful hint from @dra27 I put up this PR on their WIPocaml-5
branch. With this fix, I am able to build and installjs_of_ocaml
on 5.0~alpha -- it is for this reason that I have modifiedjs_of_ocaml
here to point to my fork.During this investigation I also found a minor issue that was causing
integers_stubs_js
to not build and I fixed this with this PRThis is my first foray at contributing to the Great OCaml5 Effort - hopefully I've got the various moving parts in the right spots so that
integers_stubs_js
can go green across the board on opam-health-check