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-render-docs-redirects: init #357383

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

GetPsyched
Copy link
Member

A helper command to manipulate the redirects mapping to avoid having to do it manually.

As of writing, this is still a WIP, and I have mentioned a few concerns as review comments to make the discussion async.

Copy link
Member Author

@GetPsyched GetPsyched left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my initial draft which only tests the happy paths. I'll be hacking on ways to make this more robust while leaving this here to marinate.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/69

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/splitting-the-nixpkgs-and-nixos-manuals-into-pages/55466/9

@GetPsyched GetPsyched mentioned this pull request Nov 19, 2024
13 tasks
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should start with tests to specify what the interaction is supposed to be like, see detailed comments

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation labels Nov 23, 2024
@fricklerhandwerk fricklerhandwerk marked this pull request as ready for review November 27, 2024 13:44

Moved existing content to a different output path?
redirects move-content <identifier> <path>

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found an unspecified requirement: nrd will complain if you rename an identifier but don't also rename references. It would be very helpful if this helper tool also did something akin to

sed -i 's/<old>/<new>/g' **/*

but that needs a bit of smarts, because we have to capture old identifiers correctly (e.g. naively inserting foo and foo2 after already having renamed the "tag" location of foo but not the "ref" locations, will produce foo22 at the "tag").

So this is not blocking for this PR. You would get the identifier errors by nrd already, so this change doesn't make it strictly worse. It may mess with expectations though, because one may think such a command will just do the Right Thing for you already.

Co-authored-by: Valentin Gagarin <valentin@gagarin.work>
@fricklerhandwerk fricklerhandwerk merged commit 836d207 into NixOS:master Nov 29, 2024
35 of 36 checks passed
@fricklerhandwerk
Copy link
Contributor

Merging, let's do further improvements in follow-up PRs.

@GetPsyched GetPsyched deleted the redirects-command branch November 29, 2024 15:32
GaetanLepage pushed a commit to GaetanLepage/nixpkgs that referenced this pull request Nov 29, 2024
Co-authored-by: Valentin Gagarin <valentin@gagarin.work>
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 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants