-
Notifications
You must be signed in to change notification settings - Fork 1
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
Generate docs for both reason & ocaml syntax #7
Conversation
2470b8c
to
c90e516
Compare
@@ -38,7 +38,7 @@ jobs: | |||
- name: Setup Node | |||
uses: actions/setup-node@v3 | |||
with: | |||
node-version: 18 | |||
node-version: 20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will silence the deprecation warnings about old Node version
@@ -4,6 +4,8 @@ DUNE = opam exec -- dune | |||
|
|||
.DEFAULT_GOAL := help | |||
|
|||
DUNE_BUILD_DIR := $(shell pwd)/_build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set this so that the build directory is the same in CI even though no local switch is being used
|
||
.PHONY: docs | ||
docs: ## Build the docs | ||
$(DUNE) build @doc | ||
rm -rf _docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish this could be a single command but I don't think it's possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a way to do this in a single command by leveraging Dune context, profiles and env vars.
In a new dune-workspace
, add this:
(context (default (profile ml)))
(context (reason (profile re)))
; define env vars
(env
(ml
(env-vars
(ODOC_SYNTAX ocaml))))
(env
(re
(env-vars
(ODOC_SYNTAX reason))))
Then, a single run of dune build @doc
should put the OCaml docs in _build/default/_doc/_html
and the Reason ones in _build/reason/_doc/_html/
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to get this to work, but ran into too many errors. Here's the last version I got before giving up:
(lang dune 3.8)
(context
(default
(profile ml)))
(context
(reason
(profile re)))
; define env vars
(env
(ml
(env-vars
(ODOC_SYNTAX ocaml)))
(re
(env-vars
(ODOC_SYNTAX reason))))
Running opam exec -- dune build @doc
gave this error:
File "dune-workspace", line 8, characters 2-8:
8 | (reason
^^^^^^
Error: Unknown constructor reason
make: *** [Makefile:59: docs] Error 1
@@ -0,0 +1,20 @@ | |||
<!DOCTYPE html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to generate this page from mld file but I couldn't figure it out how to make it work well. The problem is that I always got some navigation elements at the top that I didn't want.
@@ -14,6 +14,7 @@ | |||
(rule | |||
(alias runtest) | |||
(deps | |||
(alias_rec main) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, what's the reason for using alias_rec
instead of just alias
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is the way Melange-Dune integration works and how aliases are attached to folder in Dune.
When dune finds a melange.emit
stanza, it will create a lot of rules that generate all the artifacts needed for melange compilation (cmj
, js
and such). These rules are created into some inner folders in _build/default
, and they get also "attached" to the alias of the melange.emit
stanza (or the melange
alias if undefined).
If we don't use alias_rec
then some of these rules wouldn't execute and most probably the dependency wouldn't work properly. In other words alias_rec
is the way to tell Dune that the tests depend on any rule attached to this alias in the current folder, or any of its subfolders.
(** Initialize the Node environment to make it ready to run tests *) | ||
let init () = | ||
(* this function doesn't do anything, it just allows other modules to load the raw code below *) | ||
() | ||
|
||
[%%mel.raw | ||
{| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jchavarri This JS code below, was it copied from somewhere? If so, maybe we should add a link to the source?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! great point, I got it from here (I think): https://samthor.au/2022/test-react-builtin/
Feel free to add it to this PR or otherwise I can do it in a subsequent one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added link in cc56e26
c90e516
to
b323e52
Compare
b323e52
to
35dd3d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks so much for improving the readme ❤️
(** Initialize the Node environment to make it ready to run tests *) | ||
let init () = | ||
(* this function doesn't do anything, it just allows other modules to load the raw code below *) | ||
() | ||
|
||
[%%mel.raw | ||
{| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! great point, I got it from here (I think): https://samthor.au/2022/test-react-builtin/
Feel free to add it to this PR or otherwise I can do it in a subsequent one.
@@ -14,6 +14,7 @@ | |||
(rule | |||
(alias runtest) | |||
(deps | |||
(alias_rec main) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is the way Melange-Dune integration works and how aliases are attached to folder in Dune.
When dune finds a melange.emit
stanza, it will create a lot of rules that generate all the artifacts needed for melange compilation (cmj
, js
and such). These rules are created into some inner folders in _build/default
, and they get also "attached" to the alias of the melange.emit
stanza (or the melange
alias if undefined).
If we don't use alias_rec
then some of these rules wouldn't execute and most probably the dependency wouldn't work properly. In other words alias_rec
is the way to tell Dune that the tests depend on any rule attached to this alias in the current folder, or any of its subfolders.
You can view the docs at https://ahrefs.github.io/melange-fest/ already