Skip to content

Don't remove the doc of inline includes #595

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 26, 2021

Conversation

Julow
Copy link
Collaborator

@Julow Julow commented Feb 22, 2021

Fixes the initial issue of #478 but doesn't implement @lpw25's suggestion.

The first doc-comment of the expansion of an include was always removed. This PR avoids this for @inline includes.

The first two commits are unrelated.

@Julow Julow force-pushed the inline-include-doc branch from 94c7be8 to e80b926 Compare February 22, 2021 16:19
@jonludlam
Copy link
Member

Are you intending to implement the rest of the suggested fix?

@Julow
Copy link
Collaborator Author

Julow commented Feb 24, 2021

Not in this PR.
@lpw25 Do you think we should change the way documentation in signatures work before 2.0.0 ? (like you suggested here: #478 (comment))

@lpw25
Copy link
Contributor

lpw25 commented Feb 24, 2021

I'm not entirely clear on the status after this PR, but I do think that we should have the rules for where to put comments settled before 2.0.0. We're about to ask people to update their docs to match the new rules to get good docs.ocaml.org output, and I think we should try to avoid having to ask them twice.

I think the important thing for this bit is that you should be able to add a preamble comment to a module like:

https://github.com/janestreet/base/blob/master/src/hash.mli

by adding a first comment to the associated module type here:

https://github.com/janestreet/base/blob/master/src/hash_intf.ml#L144

@Julow Julow force-pushed the inline-include-doc branch from e80b926 to 2abaed2 Compare February 25, 2021 18:18
This makes adding a new test slightly less painful by:
- Don't require 'expected' files to be present. Promote will create them.
- Add a 'make promote-html' rule.
  This rule still promote one file at a time but it abstracts the
  _scratch/actual thing and works with the first goal.
@Julow Julow force-pushed the inline-include-doc branch from 2abaed2 to 95237ca Compare February 25, 2021 18:33
@jonludlam
Copy link
Member

It's likely we'll revisit this code with the other module doc changes, but we'll merge this now for the tests.

@Julow
Copy link
Collaborator Author

Julow commented Mar 10, 2021

I think the important thing for this bit is that you should be able to add a preamble comment to a module like:

This should work fine since #627

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.

3 participants