Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Included a js_of_ocaml version of superbol in the .vsix #93
base: master
Are you sure you want to change the base?
Included a js_of_ocaml version of superbol in the .vsix #93
Changes from 1 commit
e737563
4f4a4a1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why not
_out/$(SERVER_PROJECT).bc.js
as well?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.
Good catch, I simply forgot :)
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.
Note this conflicts with pending edits in #72 .
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.
Please investigate whether that can be achieved via fields of the corresponding
package.toml
. Same forsuperbol_free_lib/dune
.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 have changed the drom files. Running
drom build
generates a bunch ofmain.ml
files everywhere tho, not sure what's up about that. Not included in this commit, in spite of drom "helpfully" (not at all) deciding to add its changes to the index.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.
Might depend on
drom
's version. I typically useopam exec -- drom project
to generated the files instead.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, too, hate the way
drom
interacts withgit
. (Fabrice mentioned he welcomed PRs ondrom
, notably to disable auto-indexing).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 think in this case there is a
dune.drom-tpl
to edit instead.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 don't think this is a change I made —
drom
modified this file.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 saw something related to
js_of_ocaml
so I though it was edited manually. We should probably check those lines were not introduced on purpose.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.
We should ask @ddeclerck they were introduced in a commit "fix cross-compilation" (09b9ce7)
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
dune.drom-tpl
is based onhttps://github.com/OCamlPro/drom-share/blob/master/packages/js_program/dune_
. I had to add it so I could add a missing!(package-dune-stanzas)
(which comes from thepackage.toml
file and disables compiling the VSCode extension when cross-compiling : js_of_ocaml is neither available nor needed in cross-compilation environments). Obviously, this is a temporary solution ; the template dune file should be modified in the aforementioneddrom-share
repository. If someone feels like contributing todrom
.