-
Notifications
You must be signed in to change notification settings - Fork 193
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
meson: add htmldocs option for installing the docs #1269
Conversation
Perhaps it should default to false? Currently, the CI fails as sphinx-build is not installed. |
Indeed - sphinx is available in only a limited subset of the build combinations. Switched it off by default and tweaked the current documentation stage to explicitly build it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1269 +/- ##
==========================================
- Coverage 79.90% 79.90% -0.01%
==========================================
Files 67 67
Lines 19895 19892 -3
==========================================
- Hits 15897 15894 -3
Misses 3998 3998 ☔ View full report in Codecov by Sentry. |
This removes the ability to build the documentation without building the code. Can't we have both at once when doing something like index 31b99cef..2db1ba01 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -1,4 +1,4 @@
-sphinx = find_program('sphinx-build', required: false)
+sphinx = find_program('sphinx-build', required: get_option('htmldocs'))
if not sphinx.found()
subdir_done()
@@ -27,5 +27,7 @@ custom_target(
output: 'html',
depend_files: sources_doc,
command: [sphinx, '-b', 'html', meson.current_source_dir(), meson.current_build_dir() / 'html'],
- build_by_default: false
+ build_by_default: get_option('htmldocs'),
+ install_dir: docsdir,
+ install: get_option('htmldocs')
) ? What I am not yet sure about is if we should call this option |
Thanks for the prompt response team 👍 Don't recall seeing a distinct build and install, although it sounds quite reasonable. The existing non-html docs (man, conf) seem miniscule at 16K. That said, I have no opinion on the toggle name - Let me know your preferences and I'll update the PR. |
I thought more about potential additional documentation to generate like config examples, additional man pages, etc.
Let's stick to the name you used ( |
Thanks - PR updated. |
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.
Just two minor nitpicks. The rest looks fine for me!
(I have no really strong opinion on having the patch history in the commit message. Just wanted to note that this is uncommon for other PRs so far).
Currently there is a limited set of documentation installed by default. We already have the sphinx docs wired into the meson build, although there is no way for the end-user to install them. Add an `htmldocs` option which guards both build and install of the docs. Thus people can opt out of, even if they have sphinx installed. v2: - disable by default - tweak CI doc stage, to build a minimal almost docs-only build v3: - allow building, without forcing the installation v4: - capitalise HTML in meson_options - trim down meson setup command Issue: rauc#1251 Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
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.
Thank you for the changes!
Currently there is a limited set of documentation installed by default. We already have the sphinx docs wired into the meson build, although there is no way for the end-user to install them.
Add an
htmldocs
option which guards both build and install of the docs. Thus people can opt out of, even if they have sphinx installed.Issue: #1251
Thanks for wiring up sphinx to the meson build team. I've secretly waited for the autotools build to be merged, since wiring that wasn't fun :-P
Silliness aside, this PR addresses half the referenced issue hence why I didn't use "Fixes".