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

Build documentation for non-public libraries #306

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

MiloDavis
Copy link
Contributor

Currently, Jbuilder doesn't build documentation for non-public libraries. It would be nice if this was possible. This PR makes it the default, which may not be a good long-term decision. I'm happy to make any changes to improve the ergonomics and not break existing use cases.

@rgrinberg rgrinberg requested a review from trefis November 4, 2017 13:04
@rgrinberg
Copy link
Member

Am I correct to state that the private libraries will not be included in the top level generated index? So the only downside of this RP is that it makes doc generation slower because the non public docs are also built. If that's the case, I think that's a fine change.

Let's see what @trefis says about this.

@trefis
Copy link
Collaborator

trefis commented Nov 6, 2017

Am I correct to state that the private libraries will not be included in the top level generated index?

I think that's correct.

The change looks fine to me.

Copy link
Collaborator

@trefis trefis left a comment

Choose a reason for hiding this comment

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

I don't know why Jérémie chose to not generate doc for non public libraries, so perhaps having a way (clflag?) to not generate them would be nice.

Apart from that the change looks fine.

@MiloDavis
Copy link
Contributor Author

Could you merge this? We're currently using the dev version of jbuilder for Tezos and this would help us out a lot with generating documentation for some new people. If not, let me know if there are changes I can make to help facilitate this getting merged.

@rgrinberg
Copy link
Member

rgrinberg commented Nov 7, 2017 via email

@MiloDavis
Copy link
Contributor Author

@rgrinberg Is it the core cla or is there a jbuilder specific one? As of a few minutes ago, I've sent in the one for core as specified here.

@rgrinberg
Copy link
Member

Signing one is sufficient I believe. Also, I don't have a list of people who have signed the CLA so I also have to double check.

I'm merging and if there are some CLA objections from JST employees. we can revert.

@rgrinberg rgrinberg merged commit 8718fda into ocaml:master Nov 7, 2017
@MiloDavis MiloDavis deleted the doc-non-public branch November 7, 2017 13:05
@trefis
Copy link
Collaborator

trefis commented Nov 7, 2017

(Just to confirm: Milo signed the CLA)

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