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

doc: Move glob syntax to dune-glob (#7988) #10163

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

moyodiallo
Copy link
Collaborator

@moyodiallo moyodiallo commented Feb 28, 2024

Fixes #7988

@moyodiallo moyodiallo requested a review from emillon February 28, 2024 11:50
@emillon
Copy link
Collaborator

emillon commented Feb 28, 2024

Thanks.

  • This requires odoc >= 2.3.0 so I'm not sure if it will rendered properly by ocaml.org. @sabine do you know?
  • (we'll also need to mention that in dune-glob's opam file)
  • We can probably link to the docs from dependency-spec.rst instead of duplicating this information.

@moyodiallo
Copy link
Collaborator Author

We can probably link to the docs from dependency-spec.rst instead of duplicating this information.

If it's rendered properly IMHO we could go with that. If I'm not mistaken, at some point it could end up as broken link.

dune-project Outdated
@@ -131,6 +131,7 @@ execution of the action.
(stdune (= :version))
dyn
ordering
(odoc (>= 2.3.0))
Copy link
Collaborator Author

@moyodiallo moyodiallo Feb 28, 2024

Choose a reason for hiding this comment

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

I don't think It is a good idea to have dune-glob and dune packages, exclusive regarding odoc version. I don't know what is behind constraining odoc to 2.0.1 in dune package.

@Alizter
Copy link
Collaborator

Alizter commented Feb 29, 2024

Doesn't this add odoc as a build dependency? Is that really what is needed? Shouldn't it still be with-doc?

@moyodiallo
Copy link
Collaborator Author

Doesn't this add odoc as a build dependency? Is that really what is needed? Shouldn't it still be with-doc?

Indeed it should be with-doc. Changed in d79aa82

@sabine
Copy link

sabine commented Mar 16, 2024

This requires odoc >= 2.3.0 so I'm not sure if it will rendered properly by ocaml.org. @sabine do you know?

if it requires odoc >= 2.3.0, this will require ocaml-doc/voodoo#128 to merge first. This patch is currently on https://staging.docs.ci.ocamllabs.io/ and someone needs to check if all the packages build as expected after the odoc upgrade - only after that, we can merge ocaml-doc/voodoo#128.

ETA: a list of failing packages is here: https://staging.docs.ci.ocamllabs.io/package

Tezos packages failing is normal. I restarted all failed jobs.

@emillon
Copy link
Collaborator

emillon commented Mar 18, 2024

Thanks @sabine ! I subscribed to the PR and will hold this one until it's merged.

@moyodiallo
Copy link
Collaborator Author

Thanks @sabine for the merge of ocaml-doc/voodoo#128.

You could start the review @emillon.

@sabine
Copy link

sabine commented Mar 27, 2024

That the pull request on voodoo has been merged does not mean that the deployment is successful. The voodoo patch has only been tested on all the newest versions of all the packages.

It's okay to go ahead with reviewing this, but I would ask you to wait with a merge until after we know that the deployment is successful. For reference, the previous voodoo version succeeded to build documentation for 22939 packages and failed to build the documentation of 808 packages.

The documentation pipeline at https://docs.ci.ocaml.org will start in roughly an hour to build all package documentation with the new voodoo version.

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.

move glob syntax doc to dune-glob
4 participants