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: make accessible the documentation generated by odoc. #404

Merged
merged 3 commits into from
Sep 26, 2021

Conversation

EmileRolley
Copy link
Collaborator

@EmileRolley EmileRolley commented Jul 15, 2021

This PR fix some errors in documentations markups and simplify the access to the generated documentation.

Changelog

  • Fix odoc markups in some .mli files.
  • Add the src/index.mld file.
  • The make doc command now create a symbolic link from docs/odoc.html to the generated documentation index file. Currently, the only two supported modules are Test_lib and Learnocaml_report.

@EmileRolley EmileRolley requested a review from yurug July 15, 2021 14:06
@EmileRolley EmileRolley changed the title Fix: make accessible the documentation generated by odoc. Doc: make accessible the documentation generated by odoc. Jul 15, 2021
@erikmd
Copy link
Member

erikmd commented Sep 15, 2021

Hi @EmileRolley :)
given the recent merges, your PR will needed to be rebased. Will you have the time to do it? no hurry anyway.

BTW, could you elaborate on the third item of your initial post?

The make doc command now create a symbolic link from docs/odoc.html to the generated documentation index file.

In particular, do you have suggestions to increase the availability of the doc? (e.g., I would think of mentioning the make doc command in the Markdown documentation, and/or deploying the doc automatically from the CI, maybe?)

@EmileRolley
Copy link
Collaborator Author

Hi @erikmd,

given the recent merges, your PR will needed to be rebased. Will you have the time to do it? no hurry anyway.

Sure, I'm on it!

BTW, could you elaborate on the third item of your initial post?

I have specified the supported modules, but I'm not sure that's what you were talking about 😕

In particular, do you have suggestions to increase the availability of the doc? (e.g., I would think of mentioning the make doc command in the Markdown documentation, and/or deploying the doc automatically from the CI, maybe?)

Adding a comment about the make doc command in the Markdown documentation seems fine to me. However, updating the CI to deploying it doesn't seem very worth it.. Indeed, this documentation will be mainly used by developers contributing to the learn-ocaml development (which will have already cloned the repo).

@EmileRolley EmileRolley force-pushed the fix-odoc-markup branch 3 times, most recently from 02b4b5c to 0e5e6aa Compare September 16, 2021 19:03
@EmileRolley
Copy link
Collaborator Author

I added a note in the howto-setup-exercise-development-environment.md page.

I also resolved conflicts but I must have missed something, indeed, the CI is broken. It seems related to the new ocaml version...
Do you have any idea @AltGr (or @erikmd) of how it can be solved?

@erikmd
Copy link
Member

erikmd commented Sep 17, 2021

I also resolved conflicts but I must have missed something, indeed, the CI is broken. It seems related to the new ocaml version...
Do you have any idea @AltGr (or @erikmd) of how it can be solved?

Indeed I'm puzzled to see that this run failed while it was very OK during the similar CI run on master…

FTR, the corresponding log was:

+ opam switch create . ocaml-system --deps-only --locked
[NOTE] It seems you have not updated your repositories for a while. Consider updating them with:
       opam update


<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
Switch invariant: ["ocaml-system"]
Error:  Could not determine which packages to install for this switch:
  * Incompatible packages:
    - (invariant) -> ocaml-system
    - learn-ocaml -> ocaml-base-compiler = 4.12.0
    You can temporarily relax the switch invariant with `--update-invariant'
…

@AltGr do you have an idea of what's happening?

Side question: I don't know what is OCamlPro's update policy for the Docker Hub repo ocamlpro/ocaml, but I was wondering if the following code line shouldn't be changed to just docker run --rm -i ocamlpro/ocaml:4.12?

docker run --rm -i \
ocamlpro/ocaml:4.12-2021-07-25 \

unless you believe it's more stable to keep this image as is, but maybe to run opam update -y? (as suggested above in the log…)

@erikmd erikmd mentioned this pull request Sep 25, 2021
Copy link
Member

@erikmd erikmd left a comment

Choose a reason for hiding this comment

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

Hi @EmileRolley, I believe the CI issue you had spotted in this PR is now solved (in master);
but there's now a small conflict, so could you please rebase your PR on the current master?
BTW I also left one comment, for a line that should be removed.

@erikmd erikmd added this to the learn-ocaml 0.13 milestone Sep 26, 2021
@EmileRolley
Copy link
Collaborator Author

Hi @EmileRolley, I believe the CI issue you had spotted in this PR is now solved (in master);
but there's now a small conflict, so could you please rebase your PR on the current master?
BTW I also left one comment, for a line that should be removed.

Hi @erikmd, it's done.
However, I don't know how to resolve your requested change, indeed, I pushed it after rabasing it with master but it still requested...

@erikmd
Copy link
Member

erikmd commented Sep 26, 2021

Hi @EmileRolley, thanks a lot for your rebase!

However, I don't know how to resolve your requested change, indeed, I pushed it after rabasing it with master but it still requested...

actually I don't see anymore (in the PR diff) the line that I was talking about, so I guess that's very fine as is!

@erikmd
Copy link
Member

erikmd commented Sep 26, 2021

Side note: I'm puzzled to see that no CI run seems to be triggered by your last push… maybe an issue from GitHub side?

@EmileRolley
Copy link
Collaborator Author

Side note: I'm puzzled to see that no CI run seems to be triggered by your last push… maybe an issue from GitHub side?

I think it was waiting for me to accept your requested change.

@erikmd
Copy link
Member

erikmd commented Sep 26, 2021

I dunno… let's tentatively close-then-reopen your PR (as this had worked for restarting the GH workflows for #422 :)

@erikmd erikmd closed this Sep 26, 2021
@erikmd erikmd reopened this Sep 26, 2021
@erikmd erikmd merged commit e10975f into ocaml-sf:master Sep 26, 2021
@erikmd erikmd self-assigned this Sep 26, 2021
@erikmd erikmd mentioned this pull request Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants