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
vollkorn: init at 4.105 #83028
vollkorn: init at 4.105 #83028
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.
Reviewed points
- package path fits guidelines
- package name fits guidelines
- package version fits guidelines
- package build on x86_64-linux
-
meta.description
is set and fits guidelines -
meta.license
fits upstream license -
meta.platforms
is set -
meta.maintainers
is set - source is fetched using the appropriate function
- phases are respected
pkgs/data/fonts/vollkorn/default.nix
Outdated
downloadToTemp = true; | ||
recursiveHash = true; |
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.
downloadToTemp = true; | |
recursiveHash = true; |
No need to specify those, they are set by default in fetchzip.
|
||
sha256 = "0pvy72v8wsnghww67vy8i1wyk4nikyh8pylk7gjp21ar754sih3k"; | ||
|
||
meta = with lib; { |
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.
A package needs a maintainer
:)
Please add yourself (as you created the package), and also add yourself to maintainers/maintainer-list.nix in a separate commit.
@puzzlewolf updated, thanks! |
@GrahamcOfBorg eval |
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.
LGTM 👍
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.
Oh, I just noticed something:
$ ls share/doc/vollkorn-4.105/ -a
. .. ._Fontlog.txt Fontlog.txt ._OFL-FAQ.txt OFL-FAQ.txt OFL.txt
I don't think we want ._Fontlog.txt
and ._OFL-FAQ.txt
:
$ file -s share/doc/vollkorn-4.105/._*
share/doc/vollkorn-4.105/._Fontlog.txt: AppleDouble encoded Macintosh file
share/doc/vollkorn-4.105/._OFL-FAQ.txt: AppleDouble encoded Macintosh file
Bonus points for reporting upstream, I doubt they should be in the release .zip :)
We need to re-package vollkorn to use mkDerivation instead of fetchzip directly (as pointed in #83537). I stopped using this font so I have little motivation to do that and I'm closing this PR. If anyone wants vollkorn packaged for nixpkgs, just ping me here and I'll do that. |
Motivation for this change
I want to use this font.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
nix path-info -S
before and after)