Update and fix docs #354
Update and fix docs #354
Conversation
There was a problem hiding this comment.
Thanks @gabalafou
This looks great on https://lumino--354.org.readthedocs.build/en/354/
Just one comment (see code) and a suggestion: it will be nice to switch to pydata-sphinx-theme to align with the other Jupyter projects (see example in JupyterLab migration). But that could be a follow-up.
| @@ -84,7 +84,7 @@ | |||
| # | |||
| # This is also used if you do content translation via gettext catalogs. | |||
| # Usually you set "language" from the command line for these cases. | |||
| language = None | |||
| language = 'en' | |||
There was a problem hiding this comment.
Sphinx was emitting a warning about language being None.
|
|
||
| if osp.exists(api_index): | ||
| # avoid rebuilding docs because it takes forever | ||
| # `make clean` to force a rebuild |
There was a problem hiding this comment.
This didn't actually work because make clean only removes files from the build/ directory. But this code was checking the source/ directory.
This compilation-bypass was basically copied from the JupyterLab repo, but the JupyterLab repo adds additional work to make clean.
I don't think building the docs and examples for Lumino take that long, so I just decided to remove the bypass for simplicity (here and examples below).
| {% else %} | ||
| <a href="https://{{ github_host|default("github.com") }}/{{ github_user }}/{{ github_repo }}/{{ theme_vcs_pageview_mode or "blob" }}/{{ github_version }}{{ conf_py_path }}{{ pagename }}{{ suffix }}" class="fa fa-github"> {{ _('Edit on GitHub') }}</a> |
There was a problem hiding this comment.
I didn't see the point in keeping all of this extra code for BitBucket and GitLab.
| @@ -10,31 +10,11 @@ | |||
| {% block breadcrumbs_aside %} | |||
| <li class="wy-breadcrumbs-aside"> | |||
| {% if hasdoc(pagename) %} | |||
| {% if display_github %} | |||
There was a problem hiding this comment.
Didn't see the point in this if-statement.
| @@ -14,10 +14,3 @@ Lumino | |||
| api | |||
| examples | |||
| migration | |||
|
|
|||
| Indices and tables | |||
There was a problem hiding this comment.
These links come from the Sphinx getting started template, but they're not useful for this repo.
| @@ -29,7 +29,6 @@ | |||
| "build:test": "tsc --build tests && cd tests && webpack", | |||
| "clean": "rimraf ./lib && rimraf *.tsbuildinfo && rimraf ./types && rimraf ./dist", | |||
| "clean:test": "rimraf tests/build", | |||
| "docs": "typedoc --options ../../typedoc.js src", | |||
There was a problem hiding this comment.
I decided to get rid of the docs script in each individual package because it doesn't really make sense to me to build the docs for a single package in isolation of the others, especially since some of them depend on others in the monorepo.
Please let me know if my reasoning here is wrong.
| @@ -39,6 +38,10 @@ | |||
| "test:ie": "cd tests && karma start --browsers=IE", | |||
| "watch": "tsc --build --watch" | |||
| }, | |||
| "typedoc": { | |||
| "entryPoint": "./src/index.ts", | |||
| "displayName": "algorithm" | |||
There was a problem hiding this comment.
Without setting display name like this, the breadcrumbs in the rendered docs end up looking like: @lumino / @lumino/algorithm / member. You'll have to check the rendered docs, and see for yourself if you like it prefer it this way or not.
| name: '@lumino', | ||
| out: 'docs/source/api', | ||
| readme: 'README.md', | ||
| tsconfig: 'tsconfigdoc.json' | ||
| readme: 'none' |
There was a problem hiding this comment.
Setting readme to none means that the index page shows the list of packages rather than the repo readme, which allows us to get rid of the index-copy step in the conf.py file.
There was a problem hiding this comment.
I found the package first page for this PR: https://lumino--354.org.readthedocs.build/en/354/api/modules/algorithm.html
much better than the current one: https://lumino.readthedocs.io/en/stable/api/algorithm/index.html
So definitely 💯 for this new setup
|
|
||
| dest_dir = osp.join(out_dir, "api") | ||
| print(f"Copying {docs_api} -> {dest_dir}") | ||
| if osp.exists(dest_dir): | ||
| shutil.rmtree(dest_dir) | ||
| shutil.copytree(docs_api, dest_dir) | ||
| shutil.copy(osp.join(HERE, 'api_index.html'), osp.join(dest_dir, 'index.html')) |
There was a problem hiding this comment.
If we set readme to none in the Typedoc config file then we don't need to do this copy step.
|
Congrats on your first merged pull request in this project! 🎉 |
This PR contains changes to improve and fix the docs.
You can see the results of these changes by going to: https://gabalafoulumino.readthedocs.io.
You should also be able to build and try the docs locally:
conda env create -f docs/environment.yml -y conda activate lumino_documentation cd docs make html open build/html/index.html