Skip to content

Remove mdx #591

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

Merged
merged 4 commits into from
Feb 16, 2021
Merged

Remove mdx #591

merged 4 commits into from
Feb 16, 2021

Conversation

jonludlam
Copy link
Member

@jonludlam jonludlam commented Feb 15, 2021

mdx currently requires the comment parser that is part of odoc, and until this commit, odoc required mdx as a test dependeny. This PR is to remove odoc's dependency on mdx until the parser can be extracted into a separate repository. The intention is to re-add the mdx requirement as soon as the circular dependency problem is resolved, hence I have left disabled dune files in place where that made sense.

We'll re-enable them after we've split the parser out of odoc and
removed the cyclic dependency with mdx.

Signed-off-by: Jon Ludlam <[email protected]>
This was failing in the github actions tests, as bisect_ppx 2.6.0 has
a test dependency on ocamlformat 0.16.0, which has a dependency on odoc.
I assume ocaml-ci doesn't install test dependencies for the main project
dependencies. Another problem due to the ocamldoc comment parser being
part of odoc.

Signed-off-by: Jon Ludlam <[email protected]>
The esy workflow was failing to find the odoc binary, despite having
built it. I'm unsure what the cause of this is, and the workaround
is suspicious, but does seems to work.

Signed-off-by: Jon Ludlam <[email protected]>
@jonludlam
Copy link
Member Author

It'd be really useful to get someone more familiar with esy to check this over - in particular this seemed suspect to me.

@@ -104,4 +94,4 @@ jobs:
run: esy install --verbose

- name: Build odoc
run: esy odoc --version
run: esy x dune exec src/odoc/bin/main.exe -- --version
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem right. I managed to make it work with this: Julow@cd8fb9b (esy build result)

Copy link
Member Author

Choose a reason for hiding this comment

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

That looks much better - thanks!

@jonludlam jonludlam merged commit 148f0a1 into ocaml:master Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants