-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
stdlib: put all sources in same folder on installation #526
Conversation
jscomp/stdlib-412/dune
Outdated
@@ -1,4 +1,4 @@ | |||
(include_subdirs unqualified) | |||
(copy_files# ./stdlib_modules/*.ml{,i}) |
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 am not sure the #
is doing anything in this case. They point to e.g.
# 1 "jscomp/stdlib-412/stdlib_modules/weak.ml"
But the file is in _opam/lib/melange/weak.ml
—due to the further copying in (dir melange)
rule specified in lib/dune
— so it can't find the reference.
I think we can just use copy_files
?
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.
yup, fine to use copy_files
.
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.
Looks good, tests need to be promoted.
done in 0f0851e. |
Merlin struggles going to definition of submodules of the
Stdlib
because they are placed in a different folder (stdlib_modules
) than the wrap moduleStdlib
, which is one folder up.This PR changes that so that all sources of
Stdlib
are placed in the same module. Rather than moving the files up, we usecopy_files#
for backwards compat with bsconfig workflows (@anmonteiro dixit).