-
Notifications
You must be signed in to change notification settings - Fork 177
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
Drop support for old ocaml versions #947
Conversation
We'll need to get |
match Sys.getenv "BISECT_ENABLE" with | ||
| "yes" -> "(preprocess (pps bisect_ppx))" | ||
| _ -> "" | ||
| exception _ -> "" |
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 guess with recent dune (>= 2.7.0) and bisect_ppx (2.5.0) this could be done differently; namely in the dune
files add the stanza (instrumentation (backend bisect_ppx)))
(as explained in bisect_ppx README). :)
I removed some more code, see #949 (which is against the branch here). Please have a look :) |
Thanks for the additional removals! I've left one (1) comment which we may possibly ignore and merge anyway. Otherwise looks good to me. |
After adding all the constraints, the ocaml-ci dropped build attempts on legacy compilers. (Thanks Tmsc!) However, the only compiler version where all the packages are now buildable is |
You probably should make an issue on |
@raphael-proust We have a fix for See https://github.com/tmcgilchrist/lwt/tree/drop-support-for-old-ocaml-versions for a successful build against all supported packages / ocaml versions. Note it skips lwt_domains on unsupported compilers but includes it on ocaml-4.12+domains, and will pickup OCaml 5.0 once that is available. There are 2 changes I needed to make in LWT:
I didn't change the other tests but should be modified to identify which package they belong to. |
f1ccb47
to
6d98007
Compare
I've made a merge-request (into this branch) of your branch just so I could use the GH UI to merge. Thanks a lot for that fix. |
cb44404
to
c94105e
Compare
I've added one last commit to mark other tests with what packages they correspond to. But that breaks the build because there now several I think that a follow-up PR to modernise the |
This dependency is implied by "domainslib" hence there was no failure to build.
c94105e
to
6768b8e
Compare
No one has come forward with some reason to not merge this. And the MR does restore a functioning CI. So I'm merging this. |
|
Thanks @olafhering . I've fixed the issue in the release branch and added the same fix suggestion to the release PR. |
This PR removes support for ocaml<=4.07. This is following a suggestion (from @hannesm) and a user poll on discuss.