-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
nixos/doc: dedocbookify #237557
nixos/doc: dedocbookify #237557
Conversation
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 more than good on cursory inspection, with all i-s dotted. Assuming the build succeeds, I suggest just merging it. @roberth do you want to take a look? @zmitchell this would be a major item in the monthly docs announcement, as it completes the markdown transition that has been in the works for ~2 years.
would rather hold back on that claim until the nixpkgs manual is also done, to be honest. there's still docbook parts in there, and not just as build infra but in the actual documentation as verbatim blocks. |
it's been long in the making, and with 23.05 out we can finally disable docbook option docs and default to markdown instead. this brings a massive speed boost in manual and manpage builds, so much so that we may consider enabling user module documentation by default. we don't remove the docbook support code entirely yet because it's a lot all over, and probably better removed in multiple separate changes.
no longer supported. warning when used would not be appropriate, and docbook has been on the way out for long enough that throwing an error should not be necessary either.
with literalDocBook itself gone we can also remove the support code in nixos-render-docs.
with docbook no longer supported we can default to markdown option docs. we'll keep the parameter around for a bit to not break external users who set it to true. we don't know of any users that do, so the deprecation period may be rather short for this one.
docbook is now gone and we can flip the defaults. we won't keep the command line args around (unlike the make-options-docs argument) because nixos-render-docs should not be considered an exposed API.
since we no longer use the docbook path the check there will no longer fire. add one to optionsJSON to not lose this functionality.
with everything being rendered from markdown now we no longer need to postprocess any options.xml that may be requested from elsewhere. we'll don't need to keep the module path check either since that's done by optionsJSON now.
no longer needed or useful, and may even produce false positives now that markdown is the default language for option docs.
with docbook gone and MD the default these aren't needed any more. we can't remove them yet because there's thousands of uses, but maybe some day we can.
they're no longer necessary for us and will almost definitely start to rot now (like commonmark and asciidoc outputs did previously). most existing users seem to take the docbook output and run it through pandoc to generate html, those can easily migrate to use commonmark instead. other users will hopefully pipe up when they notice that things they rely on are going away. optionsUsedDocbook has only been around for one release and only exposed to allow other places to generate warnings, so that does not deserve such precautions.
|
||
This function is no longer necessary and merely an alias of `mkAliasOptionModule`. | ||
*/ | ||
mkAliasOptionModuleMD = mkAliasOptionModule; |
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.
Should mkAliasOptionModuleMD
be marked as deprecated? I can't find information about deprecating functions in the manual, but maybe a simple wrapper which prints a deprecation message could be used here?
Ditto for the other no-op aliases.
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.
that would be a bit too soon. deprecating those would require another round through the entire modules tree to remove them again, or folks would get unhelpful deprecation warnings they cannot act upon. once that's done we can deprecate them.
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's only ~10 calls to mkAliasOptionModuleMD
in nixpkgs master. I'd say ideally we'd just replace all calls in-tree with mkAliasOptionModule
in this PR, and then do another PR to remove mkAliasOptionModuleMD
itself. Otherwise this deprecated function could stay around for a long time until someone takes the time to clean up.
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 may be uses out of tree we don't know about. keeping the alias doesn't hurt while we're not sure whether it's used anywhere else, and we'd really rather keep this pr focused on removing docbook instead of tacking on refactorings that removing docbook allows to happen.
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.
keeping the alias doesn't hurt while we're not sure whether it's used anywhere else
I wasn't suggesting removing the alias in this PR, just calling mkAliasOptionModule
instead of mkAliasOptionModuleMD
everywhere. This would mean no actual changes in functionality, while making sure mkAliasOptionModuleMD
only shows up in one place, meaning it's easy to deprecate later.
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.
we can't add loud deprecation warnings to any of these public apis before all supported nixos versions agree on what the documentation language is, otherwise we'll risk breaking out-of-tree modules, backports (official in nixpkgs and unofficial by users), and who knows what else. this applies to all functions that are now aliases or no-ops.
refactorings in nixpkgs modules are less critical, but we're not going to do any. out-of-tree modules wouldn't be affected by this, but backports would be, and we're frankly sick of refactoring docs. if you want to see this done you're totally free to open a follow-up pr once this is done, but we won't be doing it.
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.
we can't add loud deprecation warnings to any of these public apis before all supported nixos versions agree on what the documentation language is, otherwise we'll risk breaking out-of-tree modules, backports (official in nixpkgs and unofficial by users), and who knows what else. this applies to all functions that are now aliases or no-ops.
I think something must've been lost in translation, because my latest comment had nothing to do with deprecation. Basically just sed -i -e s/mkAliasOptionModuleMD/mkAliasOptionModule/g
in all files except for the mkAliasOptionModuleMD = mkAliasOptionModule;
line itself. This changes no functionality anywhere, doesn't introduce any deprecation warnings, and makes it pretty obvious that we can remove mkAliasOptionModuleMD
ASAP.
refactorings in nixpkgs modules are less critical, but we're not going to do any. out-of-tree modules wouldn't be affected by this, but backports would be, and we're frankly sick of refactoring docs. if you want to see this done you're totally free to open a follow-up pr once this is done, but we won't be doing it.
Fair enough.
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 changes no functionality anywhere, doesn't introduce any deprecation warnings, and makes it pretty obvious that we can remove
mkAliasOptionModuleMD
ASAP.
this does turn backports of newly aliased options into a trap though because 23.05 uses a different language to render the non-MD variant text, potentially causing the docs build on 23.05 to fail as a result. the manual isn't checked by CI on stable branches, so this has the potential to create channel blockers. refactoring shouldn't come with that risk.
I think something must've been lost in translation, because my latest comment had nothing to do with deprecation.
yes, but the point is that deprecation is a ways out still. worrying about how it might go now may not be the wisest use of resources, especially if the changes aren't even finalized yet.
@@ -39,12 +39,17 @@ | |||
# allow docbook option docs if `true`. only markdown documentation is allowed when set to | |||
# `false`, and a different renderer may be used with different bugs and performance | |||
# characteristics but (hopefully) indistinguishable output. | |||
, allowDocBook ? true | |||
# deprecated since 23.11. | |||
# TODO remove in a while. |
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.
Would it be reasonable to set this to be removed before the next major release (24.05)? On a more general note, is there a fixed format we can use to explicitly mark TODOs for specific releases?
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.
possibly! would wait for feedback from users on that. we don't know of any users outside of nixpkgs, and if no users complain about this changing for 23.11 then maybe we can remove it 24.05. if we follow the same process as introducing markdown support we'd have to wait until 24.11 to remove this.
On a more general note, is there a fixed format we can use to explicitly mark TODOs for specific releases?
don't know of any, and without a real process in place that wouldn't make too much sense anyway. deprecations are extremely ad-hoc, and usually not in a good way.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2023-06-15-documentation-team-meeting-notes-55/29161/1 |
lib.mdDoc was removed from nixpkgs NixOS/nixpkgs#237557
* chore: bumpup CI * chore: update locks * update flake inputs and poetry locks * typescript: 1.0.15 -> 1.0.21 * bash: fix missing kernel * c: fix missing kernel * zsh: fix missing kernel * python: fix stable kernel * haskell: attempt at fixing the build * julia: fix revision * haskell: make compiler happy * haskell: fork ihaskell kernel derivation * elm: fix kernel * refacor poetry overrides * postgresql: fix missing kernel * scala: 0.14.0-RC7 -> 0.14.0-RC8 * fix poetry overrides for template * ci: workaround for no space left on device * chore: bump flake inputs * ci: fix template test * typescript: override missing nodejs-16_x * chore: remove lib.mdDoc lib.mdDoc was removed from nixpkgs NixOS/nixpkgs#237557 * python: fix broken setuptools * ci: override template input instead of sed substitution * julia: use latest kernel 1.9.4 -> 1.11.1 --------- Co-authored-by: guangtao <gtrunsec@hardenedlinux.org>
Description of changes
with epub manuals stubbed we can actually remove docbook from the nixos manual. nixos manual renders the same except for the option that went away, nixpkgs manual renders the same except for a few function doc items that changed. we'll keep docbook options export alive for a while since there are users of it out there, but we should remove it soon to ease the maintenance burden on nixos-render-docs keeping docbook around without using it creates.
since we don't support docbook rendering any more we can also simplify the manual rendering ci jobs, and drop the rendering equality check completely.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)