-
-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
meh: init at 0.3 #24027
meh: init at 0.3 #24027
Conversation
name = "meh-0.3"; | ||
|
||
src = fetchFromGitHub { | ||
owner = "jhawthorn"; |
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.
Indentation is inconsistent.
src = fetchFromGitHub { | ||
owner = "jhawthorn"; | ||
repo = "meh"; | ||
rev = "4ab1c75f97cb70543db388b3ed99bcfb7e94c758"; |
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.
You can probably do rev = "v0.3"
here. For convenience, consider factoring out a dedicated version attribute, to be reused in name
and rev
.
|
||
outputs = [ "out" "doc" ]; | ||
|
||
buildInputs = [ libXext libX11 libjpeg libpng giflib]; |
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.
Inconsistent whitespace.
@joachifm thanks for reviewing -- I was taking a look at this and actually realized that 0.3 is from 2010 while the latest rev has had 25 commits since then -- I would add a default @ v0.3 and an unstable.nix except v0.3 doesn't compile (appears change in giflib function since that time) -- advice? maybe keep as fetchFromGitHub and use rev as version, default.nix following latest instead of release? |
Package the version that works, I guess, but only one version please, unless there's a compelling reason to have more :) Beware that revisions are unsuitable as version strings, however. See https://github.com/NixOS/nixpkgs/blob/master/doc/coding-conventions.xml#L256 |
amended based on feedback |
description = "A minimal image viewer using raw XLib"; | ||
homepage = http://www.johnhawthorn.com/meh/; | ||
license = stdenv.lib.licenses.mit; | ||
platforms = with stdenv.lib.platforms; unix; |
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.
should this work on other platforms?
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.
ah, nope. will amend and repush
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.
confirming -- if only linus appropriate edit is?:
platforms = with stdenv.lib.platforms; linux;
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.
yep or platforms = stdenv.lib.platforms.linux;
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.
it's either with foo; bar
or foo.bar
, you accidentally left the with in there
I get this on darwin
|
Motivation for this change
Add a package.
First time, read contributing and tested with nox-review, useSandbox, and resulting binaries.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)