doc: init#1891
Conversation
e495d02 to
46a04d5
Compare
|
User story created: #1893 |
6d515aa to
d81f918
Compare
c8a17b1 to
485707a
Compare
eljamm
left a comment
There was a problem hiding this comment.
I've only had a brief look at the manual entrypoint and will have a more in-depth one once these comments have been addressed and I can build it locally.
eljamm
left a comment
There was a problem hiding this comment.
There are probably more things to address, but let's go through this step by step. I've included a patch for most of the things I reviewed for convenience:
0001-Review-suggestions.patch
From ac9e7ca0a18acade0b93b1dfed4b81743ca50248 Mon Sep 17 00:00:00 2001
From: eljamm <fedi.jamoussi@protonmail.ch>
Date: Thu, 8 Jan 2026 10:13:30 +0100
Subject: [PATCH] Review suggestions
---
default.nix | 3 +-
maintainers/shells/default.nix | 11 +++
manuals/default.nix | 125 +++++++++------------------------
3 files changed, 45 insertions(+), 94 deletions(-)
diff --git a/default.nix b/default.nix
index 6dc3ac13..a52a8990 100644
--- a/default.nix
+++ b/default.nix
@@ -57,10 +57,9 @@ let
options = self.optionsDoc.optionsNix;
};
- manuals = pkgs.callPackage manuals/default.nix rec {
+ manuals = self.call manuals/default.nix rec {
version = "0.0.0"; # FixMe(maint): use something better, alas self.rev is not available there.
revision = "release-${version}";
- inherit (self.project-utils.raw-projects.config) projects;
modulesPath = "${sources.nixpkgs}/nixos/modules";
};
diff --git a/maintainers/shells/default.nix b/maintainers/shells/default.nix
index 0fc85b65..c88fc682 100644
--- a/maintainers/shells/default.nix
+++ b/maintainers/shells/default.nix
@@ -12,6 +12,17 @@ pkgs.mkShellNoCC {
buildArgs = "-A overview --show-trace -v";
})
+ (pkgs.writeShellApplication {
+ name = "devmode-manual";
+ text = ''
+ ${lib.getExe (
+ pkgs.devmode.override {
+ buildArgs = "-A manuals --show-trace -v";
+ }
+ )}
+ '';
+ })
+
(pkgs.writeShellApplication {
# TODO: have the program list available tests
name = "ngipkgs-test";
diff --git a/manuals/default.nix b/manuals/default.nix
index 928835aa..67092afd 100644
--- a/manuals/default.nix
+++ b/manuals/default.nix
@@ -1,78 +1,20 @@
{
- callPackage,
lib,
+ callPackage,
+ nixosOptionsDoc,
+ perl,
python3,
stdenv,
texlive,
- version,
- projects,
- nixosOptionsDoc,
+
modulesPath,
- pkgs,
+ nixos-modules,
+ version,
...
}:
-let
- pythonPackages = python3.withPackages (
- pyPkgs: with pyPkgs; [
- linkify-it-py
- myst-parser
- sphinx
- #sphinx-argparse
- #sphinx-autoapi
- #sphinx-autobuild
- #sphinx-autodoc-typehints
- #sphinx-autodoc2
- #sphinx-automodapi
- #sphinx-basic-ng
- #sphinx-better-theme
- sphinx-book-theme
- #sphinx-click
- #sphinx-codeautolink
- #sphinx-comments
- sphinx-copybutton
- sphinx-design
- #sphinx-external-toc
- #sphinx-favicon
- #sphinx-fortran
- #sphinx-hoverxref
- #sphinx-inline-tabs
- #sphinx-intl
- #sphinx-issues
- #sphinx-jinja
- #sphinx-jinja2-compat
- #sphinx-jquery
- #sphinx-jupyterbook-latex
- #sphinx-last-updated-by-git
- #sphinx-lv2-theme
- #sphinx-markdown-builder
- #sphinx-markdown-parser
- #sphinx-markdown-tables
- #sphinx-material
- #sphinx-mdinclude
- #sphinx-multitoc-numbering
- #sphinx-multiversion
- sphinx-notfound-page
- #sphinx-prompt
- #sphinx-pytest
- #sphinx-remove-toctrees
- #sphinx-reredirects
- #sphinx-rtd-dark-mode
- #sphinx-rtd-theme
- #sphinx-serve
- sphinx-sitemap
- #sphinx-tabs
- #sphinx-testing
- #sphinx-thebe
- #sphinx-tippy
- #sphinx-togglebutton
- #sphinx-toolbox
- #sphinx-version-warning
- #sphinx-versions
- ]
- );
-in
-stdenv.mkDerivation {
+stdenv.mkDerivation (finalAttrs: {
name = "NGIpkgs-Manuals";
+ inherit version;
src =
with lib.fileset;
toSource {
@@ -98,14 +40,9 @@ stdenv.mkDerivation {
];
};
nativeBuildInputs = [
- pythonPackages
- pkgs.perl
- # Explanation: generated with nix run github:rgri/tex2nix -- *.tex *.sty
- (callPackage ./tex-env.nix {
- extraTexPackages = {
- inherit (texlive) latexmk gnu-freefont;
- };
- })
+ finalAttrs.passthru.pythonPackages
+ finalAttrs.passthru.texEnv
+ perl
];
patchPhase = ''
substituteInPlace manuals/index.md \
@@ -127,34 +64,38 @@ stdenv.mkDerivation {
popd
'';
passthru = {
+ pythonPackages = python3.withPackages (
+ pyPkgs: with pyPkgs; [
+ linkify-it-py
+ myst-parser
+ sphinx
+ sphinx-book-theme
+ sphinx-copybutton
+ sphinx-design
+ sphinx-notfound-page
+ sphinx-sitemap
+ ]
+ );
+ # generated with: nix run github:rgri/tex2nix -- *.tex *.sty
+ texEnv = callPackage ./tex-env.nix {
+ extraTexPackages = {
+ inherit (texlive) latexmk gnu-freefont;
+ };
+ };
optionsDoc = nixosOptionsDoc {
options =
lib.flip builtins.removeAttrs [ "_module" ]
(lib.evalModules {
class = "nixos";
- specialArgs = {
- inherit pkgs modulesPath;
- };
+ specialArgs = { inherit modulesPath; };
modules = [
{
config = {
- # Explanation: do not check anything
- # because NixOS options are not included.
+ # Do not check anything because NixOS options are not included.
# See also comment in NixOS' `noCheckForDocsModule`.
_module.check = false;
};
- imports = lib.flatten (
- lib.attrValues (
- lib.mapAttrs (
- name: project:
- lib.attrValues (
- lib.filterAttrs (_: module: module != null) (
- lib.mapAttrs (n: p: p.module or null) (project.nixos.modules.services or { })
- )
- )
- ) projects
- )
- );
+ imports = lib.attrValues nixos-modules.services;
#options = (import projects/types.nix { inherit lib; }).options;
#inherit (self.project-utils.eval-projects) options;
}
@@ -162,4 +103,4 @@ stdenv.mkDerivation {
}).options;
};
};
-}
+})
--
2.51.2
There was a problem hiding this comment.
We could add a manuals devmode to shells/default.nix:
(pkgs.writeShellApplication {
name = "devmode-manuals";
text = ''
${lib.getExe (
pkgs.devmode.override {
buildArgs = "-A manuals --show-trace -v";
}
)}
'';
})There was a problem hiding this comment.
If we're introducing more dev tools I think #1880 (comment) is needed more than ever.
|
@ju1m , is it possible to split this PR to multiple smaller ones ? It is very hard to review or approve 180 files.
|
63240fa to
c14a92d
Compare
|
| manuals = self.call manuals/default.nix rec { | ||
| version = "0.0.0"; # FixMe(maint): use something better, alas self.rev is not available there. | ||
| revision = "release-${version}"; | ||
| inherit (self.project-utils.raw-projects.config) projects; |
There was a problem hiding this comment.
Not used
| inherit (self.project-utils.raw-projects.config) projects; |
|
|
||
| manuals = self.call manuals/default.nix rec { | ||
| version = "0.0.0"; # FixMe(maint): use something better, alas self.rev is not available there. | ||
| revision = "release-${version}"; |
There was a problem hiding this comment.
This doesn't seem to be used in manuals/default.nix
| # Explanation: do not check anything | ||
| # because NixOS options are not included. | ||
| # See also comment in NixOS' `noCheckForDocsModule`. | ||
| _module.check = false; |
There was a problem hiding this comment.
I've been meaning to ask, why do you always append Explanation: to comments? I think it's a bit redundant since their purpose is to explain things, in the first place.
There was a problem hiding this comment.
I gave you my rationale there
They are all kind of comments, not all of them explain things, so I classify them to ease their digesting, facilitate their browsing and keeping their content in focus.
The current taxonomy is not perfect, always up for discussion, I hope you can agree with me, but of course you have the final word.
There was a problem hiding this comment.
I gave you my rationale there
Oh, I didn't notice.
They are all kind of comments, not all of them explain things, so I classify them to ease their digesting, facilitate their browsing and keeping their content in focus.
I don't know about other contributors, but personally I find Explanation: a bit distracting, especially since comments are meant to document and explain things by default. To me, it doesn't make much sense to be explicit like that in situations where this is self-implied.
Explanation:is a classification header that I consider as important asToDo:,Warning:,Issue:,FixMe:,Description:,Documentation:.
This is surely up to preference, but the TODO-comments are usually short (FIX, WARN, NOTE, ...) that it's not so distracting for me, whereas the length and capitalization in these stick out too much, IMO.
There was a problem hiding this comment.
comments are meant to document and explain things by default.
I browsed the comments and you're perfectly right that most untagged comments
in packages and services are explanations (answering "why?")
with some exceptions here and there which are rather:
- descriptions,
- history
notes, - alternative
implementations - invariant to respect.
- mix of different concerns
- section titles
As an aside, in libraries (eg. pkgs/lib.nix or overview.utils)
untagged comments are most often descriptions (answering "what?").
Now, since having this classifier does not have the expected effect of helping you
to parse/browse/check-existence-of explaining comments,
I'll fallback to untagged comments for explanations.
For the record, it's because explanations are dignified
with their own separate category in https://diataxis.fr
that I logically introduced this new classifier
(sometimes tagged further down with a scope between parenthesis),
to nudge me to actually explain things, instead of not writing any explaination,
or just describing what the code already states.
| writeText "options.md" ( | ||
| '' | ||
| {#User_What_is_the_tree_of_options} | ||
| # What is the tree of options? | ||
|
|
||
| The following tree of options is provided by importing: | ||
| - `inputs.NGIpkgs.nixosModules.programs` | ||
| - `inputs.NGIpkgs.nixosModules.services` | ||
|
|
There was a problem hiding this comment.
Do we want/need the list of options when we already this in the overview? The biggest disadvantage to me for this approach is that the list is too long and encumbered, making it difficult to read.
I think it would be better to split this into a separate PR where we can discuss how we should present this without blocking this PR, since options aren't really essential for the manual. And ideally, we should have a user story to state what the manual is accomplishing and what value it adds to users.
What do you think @imincik?
There was a problem hiding this comment.
Where is the list of options in the overview?
Note that it could not have been generated without the fixes from #1942
The options' sections are hidden from the ToC, still available in the document ToC in a PDF viewer's side-window.
Funny, for me options are like the most essential thing I'm expecting from a User manual.
Anyway, I'll split in another PR.
There was a problem hiding this comment.
List of options in ngipkgs might not be that relevant as for example in nixpkgs. Our users will find available options listed in project instructions in ngi.nixos.org and we don't expect them to configure multiple projects at once.
But I am ok with having the list options in manual if they don't flood content.
There was a problem hiding this comment.
@ju1m , I just built the manual and I definitely don't think we should have configuration options listed on the home page as I see it now. Are you able to move them to separate (Configuration options) page ? Thanks.
| writeText "options.md" ( | ||
| '' | ||
| {#User_What_is_the_tree_of_options} | ||
| # What is the tree of options? | ||
|
|
||
| The following tree of options is provided by importing: | ||
| - `inputs.NGIpkgs.nixosModules.programs` | ||
| - `inputs.NGIpkgs.nixosModules.services` | ||
|
|
There was a problem hiding this comment.
@ju1m , I just built the manual and I definitely don't think we should have configuration options listed on the home page as I see it now. Are you able to move them to separate (Configuration options) page ? Thanks.
|
|
@ju1m , I am getting following error when trying to build manual |
|
@ju1m ,
|
|
I have just discovered that |
This PR introduces a demo of what could be used to make documentation for
NGIpkgs, currently three documents:State
Based upon:
MyST;sphinx.MySTrendition ofprogramsandservicesoptions fromoptionsNix.Build and read with:
Related
optionsDocforservicesandprograms. #1942RoadMap