-
Notifications
You must be signed in to change notification settings - Fork 17
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
[WIP] Fill in dune-project file #116
base: master
Are you sure you want to change the base?
Conversation
dune-project
Outdated
(package | ||
(name reglfw) | ||
(synopsis "Cross-platform Reason / OCaml bindings for GLFW") | ||
(description "") |
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.
Need a description 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.
Same as synopsis is good.
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.
Thanks, I've pushed your changes. It would be nice to have a more detailed description though if possible.
|
||
(source (github revery-ui/reason-glfw)) | ||
(authors "Bryan Phelps") | ||
(maintainers "Bryan Phelps <[email protected]>") |
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.
Should anybody else be added to these authors
and maintainers
fields?
You haven't run opam lint :) because the lint say we must add a . at the end of synopsys and description. And sorry I hadn't seen the message is outdated. I have added a notice now that is up to date. Revery is know to compile from 4.07 to 4.09 maybe he compile on 4.06 (so just try to compile with 4.06 if you want. If you want to quickly move on you can put this three version.) (3) We have also regular deps that are purely OCaml/ReasonML. some with Reason-smth has of course dep to reason (published on OPAM) |
But seem to be fine just need to do that for other package. |
Sorry about that, should work now.
I've put the version to >= 4.07 if that's alright with you? (Note that it seems to work with 4.06 so I can change the version to that if you want.) |
Is there a dependency we're missing in the dune-project file that's causing the Linux build to fail? It seems it "cannot find -lGL" and "cannot find -lGLU": https://dev.azure.com/revery-ui/revery/_build/results?buildId=3681&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&t=6c939d89-0d1a-51f2-8b30-091a7a82e98c&l=147. Apparently the MacOS build failed due to a permissions error, not sure why that happened: https://dev.azure.com/revery-ui/revery/_build/results?buildId=3681&view=logs&j=a5e52b91-c83f-5429-4a68-c246fc63a4f7&t=2b3990e0-410c-52b9-0fec-7c266a5d9fcd&l=14 |
We install it in ci before running esy https://github.com/revery-ui/reason-glfw/blob/master/azure-pipelines.yml#L21 |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
mac os with error 127 are back 🤔 and pull rejest. I don't know what happen. For the linux issue it seem the dune file are out of date (and seem to ignore config folder) https://github.com/revery-ui/reason-glfw/blob/master/config/discover.ml#L43-L44 |
Hmm, so maybe we use a later version of dune then? I’m not sure about getting dune to recognize the config, I’d have to look into that. |
A few things need to be filled in before this is ready for merge:
description
part of the(package
section.depends
part of the(package
section. I'm not sure what version of OCaml should used, so I've left it as(ocaml (>= <FILL VERSION HERE>))
. If anyone knows the version that should be used, let me know and I'll update this. Also, any other dependencies will need to be put here, which don't necessarily have to have versions associated with them but should be listed.If I'm forgetting any other fields (or if there are any other changes that should be made), let me know. Note that I've used https://github.com/rgrinberg/opium/blob/master/dune-project as a reference here.
The reason for this addition is so that Revery can be published on OPAM (see revery-ui/revery#71), which will require that its dependencies be published on OPAM as well (this repository was mentioned as a dependency of Revery in revery-ui/revery#71 (comment)). With this PR, the
dune-project
file will now generatereglfw.opam
(due to(generate_opam_files true)
), preparing this library for publishing on OPAM.