Conversation
Compiler Merge ChecklistThis PR seems to merge changes from Flambda. Please be sure to follow the below steps:
If this PR is not merging changes from Flambda, feel free to ignore this comment |
| val fold_modules_lazy: | ||
| (string -> Path.t -> Subst.Lazy.module_declaration -> 'a -> 'a) -> | ||
| Longident.t option -> t -> 'a -> 'a | ||
| val fold_modtypes_lazy: | ||
| (string -> Path.t -> Subst.Lazy.modtype_declaration -> 'a -> 'a) -> | ||
| Longident.t option -> t -> 'a -> 'a |
There was a problem hiding this comment.
I intend to make a compiler PR that adds these functions.
dkalinichenko-js
left a comment
dkalinichenko-js
left a comment
There was a problem hiding this comment.
Overall LGTM, but worth rebasing onto the retag.
| "pos": { | ||
| "line": 344, | ||
| "col": 0 | ||
| "col": 9 |
There was a problem hiding this comment.
What's the context for this?
There was a problem hiding this comment.
There's actually no net change here. On main this is 9 (9 is the correct value), but due to the cms thing, the column here changed to 0. When I changed the prioritization order, this changed back to 9. I just happened to promote and commit this test before making the prioritization change (see f1a8cdb).
|
The new commit looks good to me. |
2 participants
When reviewing, I skip the first 4 commits. These commits run the merge script, commit conflicts, and fix some trivial conflicts that aren't worth reviewing. But everything after f73f8de should be reviewed.
Some comments about this merge:
load_path.mlbecause of the new-I-manifestand-H-manifestparameters (Add manifest files support oxcaml#4986). After discussing with @Forestryks, we concluded that Merlin doesn't need to understand these for now.