-
-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
trenchbroom: init at 2019.6 #76607
trenchbroom: init at 2019.6 #76607
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.
Some feedback:
Missing reference to pkg in pkgs/top-level/all-packages.nix
|
||
meta = with stdenv.lib; { | ||
description = "TrenchBroom is a modern cross-platform level editor for Quake-engine based games"; | ||
homepage = http://kristianduske.com/trenchbroom; |
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.
Please quote the URL NixOS/rfcs#45
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.
homepage = http://kristianduske.com/trenchbroom; | |
homepage = "http://kristianduske.com/trenchbroom"; |
cmakeFlags = [ "-DwxWidgets_PREFIX=${wxGTK31}" ]; | ||
hardeningDisable = [ "format" ]; | ||
|
||
postInstall = "mkdir $out/share/applications; ln -s ../TrenchBroom/trenchbroom.desktop $out/share/applications"; |
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.
- Use multi line syntax for the
postInstall
script - Consider using
cp
orinstall
instead ofln
pname = "TrenchBroom"; | ||
version = "2019.6"; | ||
|
||
src = fetchgit { |
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.
use fetchFromGitHub
Thank you for your contributions. This has been automatically marked as stale because it has had no activity for 180 days. If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity. Here are suggestions that might help resolve this more quickly:
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-run-qt-apps-on-a-build-time-trying-to-pack-trenchbroom/8451/1 |
}; | ||
|
||
buildInputs = [ mesa mesa_glu freeimage freetype pandoc wxGTK31 libX11 ]; | ||
nativeBuildInputs = [ cmake git ]; |
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.
nativeBuildInputs = [ cmake git ]; | |
nativeBuildInputs = [ cmake ]; |
If git is fetching some info about the repo please patch that.
cmakeFlags = [ "-DwxWidgets_PREFIX=${wxGTK31}" ]; | ||
hardeningDisable = [ "format" ]; | ||
|
||
postInstall = "mkdir $out/share/applications; ln -s ../TrenchBroom/trenchbroom.desktop $out/share/applications"; |
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.
postInstall = "mkdir $out/share/applications; ln -s ../TrenchBroom/trenchbroom.desktop $out/share/applications"; | |
postInstall = '' | |
mkdir $out/share/applications | |
cp ../TrenchBroom/trenchbroom.desktop $out/share/applications | |
''; |
|
||
meta = with stdenv.lib; { | ||
description = "TrenchBroom is a modern cross-platform level editor for Quake-engine based games"; | ||
homepage = http://kristianduske.com/trenchbroom; |
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.
homepage = http://kristianduske.com/trenchbroom; | |
homepage = "http://kristianduske.com/trenchbroom"; |
Closing since author did not respond since opening this almost a year ago. Please fix the review comments and ping me and I'll reopen the PR. |
Motivation for this change
This is my first NixOS package, so any constructive feedback would be greatly appreciated!
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)