-
Notifications
You must be signed in to change notification settings - Fork 429
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
OCaml 4.08 support #2426
OCaml 4.08 support #2426
Conversation
src/reason-parser/reason_parser.mly
Outdated
module Clflags = struct | ||
include Clflags | ||
|
||
let fast = unsafe |
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 pretty annoying. It won't build in <4.08 so we might have to introduce yet another .cppo
compat file
(rule | ||
(targets lexer_warning.ml) | ||
(deps ../generate/select.exe lexer_warning.ml-4.07 lexer_warning.ml-4.06 lexer_warning.ml-default) | ||
(action (with-stdout-to %{targets} (run ../generate/select.exe lexer_warning.ml-4.07 lexer_warning.ml-4.06 lexer_warning.ml-default)))) | ||
(deps ../generate/select.exe lexer_warning.ml-4.08 lexer_warning.ml-4.07 |
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.
Just for your information, select.ml
is where we currently make similar compiler version specific decisions. That is an alternative to cppo. (It might not be as easy to do so for these particular special cases in this diff).
ea7890f
to
927ab0e
Compare
Woohoo! Thanks as always. |
This PR makes Reason build on OCaml 4.08.
It depends on let-def/merlin-extend#9
The 4.08 CI steps are failing because of the above dependency. Perhaps we should vendor merlin-extend too as it's such a small dep?