-
Notifications
You must be signed in to change notification settings - Fork 412
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
Restore compatibility with 4.02 #5381
Conversation
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.
Can you add a changelog entry ?
This reverts commit 3f08057. Signed-off-by: nojebar <nicolas.ojeda.bar@lexifi.com>
ac63a54
to
7e82596
Compare
I've tested a backport of this on lwt_react.1.1.5 + ocaml-base-compiler.4.02.3 which is the failure @kit-ty-kate reported. Unfortunately it doesn't work:
|
OK, let me try something else. |
Could the tests be enabled for 4.02 ? |
7e82596
to
4c00d53
Compare
I pushed an alternative fix that should work on 4.02. |
Signed-off-by: nojebar <nicolas.ojeda.bar@lexifi.com>
Signed-off-by: nojebar <nicolas.ojeda.bar@lexifi.com>
4c00d53
to
190dabc
Compare
Just thinking about this again, perhaps we want to simply pass |
The downside is that 1) we may miss deprecations if we are not warned about them, and 2) the deprecation alerts will also not be triggered for the user code. |
I confirm it works, thanks Nicolas. What to do w.r.t. "-w -3", that I'm unsure, indeed seems like a delicate tradeoof. |
OK, let's leave that for later and merge this PR as it is. |
…figurator, dune-build-info and dune-action-plugin (2.9.3) CHANGES: - Disable warning for deprecated Toploop functions used in dune files written in OCaml syntax. Restores 4.02 compatibility. (ocaml/dune#5381, @nojb)
I did some brief testing on my side, but would appreciate confirmation from some other dev (especially if this is not being tested in the CI).