Skip to content
This repository has been archived by the owner on Jun 6, 2020. It is now read-only.

Fix missing deps #70

Merged
merged 3 commits into from
May 9, 2020
Merged

Fix missing deps #70

merged 3 commits into from
May 9, 2020

Conversation

Et7f3
Copy link
Member

@Et7f3 Et7f3 commented May 9, 2020

The js sandbox need lwt_ppx that is not bundled with lwt anymore.
This is a intermediate step to #59 I.e. ensure master build then reduce deps.

@Et7f3 Et7f3 requested a review from lessp May 9, 2020 09:12
lessp
lessp reviewed May 9, 2020
View reviewed changes
Copy link
Member

@lessp lessp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find! I'm getting missing dependencies when trying to run the JS-build esy @js run. Does it work for you? Switching over to serve seems to work better on my end, might be an idea to use that instead.

@Et7f3
Copy link
Member Author

Et7f3 commented May 9, 2020

Ok it build but it doesn't run due to missing jsoo stubs.

lessp
lessp approved these changes May 9, 2020
View reviewed changes
Copy link
Member

@lessp lessp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it, builds and starts up OK (a part from the missing stubs as you said) from the JS-manifest. 👍

@Et7f3 Et7f3 merged commit 70651a2 into master May 9, 2020
@Et7f3 Et7f3 deleted the fix_missing_deps branch May 9, 2020 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lessp lessp lessp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Et7f3 @lessp