Skip to content
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/make-options-doc: use dummy store for XML conversion #211648

Closed
wants to merge 1 commit into from

Conversation

ncfavier
Copy link
Member

Mirror of https://gitlab.com/rycee/nmd/-/merge_requests/10

dummy:// was added in Nix 2.4, but since we use pkgs.nix directly this is not an issue. https://nixos.org/manual/nix/unstable/release-notes/rl-2.4.html

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 19, 2023
@pennae
Copy link
Contributor

pennae commented Jan 19, 2023

is this urgent? if #211598 lands we're going to remove optionsXML and the stylesheets that process it before too long, and that should also obviate the need for nix here

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Jan 19, 2023
@ncfavier
Copy link
Member Author

is this urgent?

Not at all, I don't think anyone has reported an issue like nix-community/home-manager#3605 in nixpkgs.

if #211598 lands

I definitely want to review that soon ❤️

@ncfavier
Copy link
Member Author

(How did you find this PR so fast 😅? Are you watching topic: nixos or something?)

@pennae
Copy link
Contributor

pennae commented Jan 19, 2023

Not at all, I don't think anyone has reported an issue like nix-community/home-manager#3605 in nixpkgs.

that's good at least. though thinking about the home-manager pr as it is, removing optionsXML is probably going to cause some disruption 🙁 guess we'll have to live with that, the removal of docbook is going to disrupt things one way or other.

I definitely want to review that soon ❤️

❤️ can't wait to be done with docbook 😬

(How did you find this PR so fast sweat_smile? Are you watching topic: nixos or something?)

no, just coincidence. we check the pull request list occasionally when we're bored, and this time you came up first :D

@ncfavier
Copy link
Member Author

removing optionsXML is probably going to cause some disruption

Well, not for home-manager since it's using NMD which has its own optionsXML. (Hence the need for this duplicate PR)

@pennae
Copy link
Contributor

pennae commented Jan 20, 2023

not for home-manager, but potentially for other users of optionsXML. hopefully those don't even exist (and we're very inclined to call optionsJSON, optionsXML etc implementation details of the docs system). but the attribute has been exposed for a while and we don't know who uses it, so there's always a risk.

in this case the risk is more likely on the side of just removing it and waiting for someone to complain than to keep it around for however long yet.

@ncfavier
Copy link
Member Author

Closing in favour of #212289.

@ncfavier ncfavier closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants