Minor Nixpkgs doc improvements#319875
Conversation
Nix eval isn't made to patch stuff, it's more flexible to do it at build time
All its files were removed years ago!
Especially when only Nix files are changed
github-actions
bot
added
the
8.has: documentation
| inherit (lib) hasPrefix removePrefix; | ||
| fs = lib.fileset; |
There was a problem hiding this comment.
@infinisil, I've seen this fs = lib.fileset idiom a couple times in your patches now -- interested in why you're doing it. fs.unions, fs.fileFilter don't look like better names to me than fileset.unions or fileset.fileFilter. I'm not strongly opposed or have something else to suggest, just that the choice doesn't yet make me go "heck yes."
There was a problem hiding this comment.
Yeah that's fair, I think it's just smaller and I'm kind of used to it by now. It's like a qualified import (use foo::bar as fb in Rust, or import qualified Foo.Bar as FB in Haskell)
There was a problem hiding this comment.
I'd say let's keep it this way though, it's not a problem (unlike the with pattern!)
There was a problem hiding this comment.
Sure thing. Side note: I'm a filesystems engineer by trade, and so fs always means file system to me ;-)
| src = fs.toSource { | ||
| root = ./.; | ||
| fileset = fs.unions [ | ||
| (fs.fileFilter (file: | ||
| file.hasExt "md" | ||
| || file.hasExt "md.in" | ||
| ) ./.) | ||
| ./style.css | ||
| ./anchor-use.js | ||
| ./anchor.min.js | ||
| ./manpage-urls.json | ||
| ]; |
There was a problem hiding this comment.
I was interested in the difference, so I built the derivation on master and on this PR:
$ $ diff -u4 -r /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc /nix/store/j1nq1anc8hc6gai6h7ib7rv9jjcs9aq5-source
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: common.nix
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: default.nix
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: doc-support
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc/functions: library
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: old
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: shell.nix
Only in /nix/store/w483x0vv260dikx0wzl49kf6zkd5i673-doc: tests
This does make me think that this would be less error-prone to exclude the following file types:
.nix.py.txt(but also: consider removingdoc/old/cross.txt.)
There was a problem hiding this comment.
I think it's cleaner to explicitly list what's needed, so that it's directly associated with the build instructions further down. Also this way we don't need to worry about new file types accidentally influencing the build.
There was a problem hiding this comment.
Right, my thought is that it's expected in docs to have css, js, and json. Sort of the converse, where there's unexpected breakage due to needing to allowlist one sort of file (css, js) but not another (md).
ofborg
bot
added
10.rebuild-darwin: 0
wegank
added
the
12.approvals: 1
5 participants
Description of changes
src = ./.entries that unintentionally include their own.nixfiles #301014Things done
nix-build docAdd a 👍 reaction to pull requests you find important.