Skip to content
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

Add package-level mlds to odoc include path #2016

Merged
merged 5 commits into from
Apr 10, 2019
Merged

Conversation

jonludlam
Copy link
Member

This ensures links to/from docs that exist outside of a library are correctly resolved.

Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

Looks good. Please update gen_tests.ml like the other odoc tests. Tests need to specify they introduce external dependencies.


$ dune build @doc

$ grep -r xref-unresolved _build/default/_doc/_html/odoc_page_link_bug/index.html
Copy link
Member

Choose a reason for hiding this comment

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

It might make sense of just build this target directly. It will make the tests a tad quicker.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good idea, fixed.

src/odoc.ml Outdated Show resolved Hide resolved
@rgrinberg
Copy link
Member

Don't forget to update the change log by the way

CHANGES.md Outdated
@@ -1,6 +1,10 @@
unreleased
----------

- Fix invocation of odoc to add previously missing include paths, impacting
mld files that are not in directories containing libraries (fixes #2007,
@jonludlam)
Copy link
Member

Choose a reason for hiding this comment

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

This should also include the PR number.

@jonludlam
Copy link
Member Author

Should I rebase/force push to resolve the out-of-date issue or hit the 'update branch' button?

@rgrinberg
Copy link
Member

I'm used to doing it manually. I think either one should work.

@jonludlam jonludlam merged commit 56c4570 into ocaml:master Apr 10, 2019
@jonludlam jonludlam deleted the fix-2007 branch April 10, 2019 20:48
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 11, 2019
CHANGES:

- Fix invocation of odoc to add previously missing include paths, impacting
  mld files that are not in directories containing libraries (ocaml/dune#2016, fixes
  ocaml/dune#2007, @jonludlam)
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