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
Add iim #91657
Add iim #91657
Conversation
I marked this as stale due to inactivity. → More info |
@monokrome : cleaned up the derivation, does this meet your intent? |
@tomberek Yeah, sorry - it was my first package and I didn't know how to access the things lol. Thank you 🙏🏼 |
What does the "This PR does not cleanly list package outputs after merging." imply? Is there a new function for listing them or something? |
Also, this isn't meant to be a draft... I don't know how that happened... |
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.
Fixed up the fetchFromGitHub, this does a more efficient fetching. Do you want to add yourself as a maintainer?
stdenv.mkDerivation { | ||
name = "iim"; | ||
|
||
src = builtins.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.
my mistake, this should be fetchFromGithub (and added to the inputs at the top
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.
@tomberek What does "added to the inputs at the top" mean here? Apologies for my confusion
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 should use fetchFromGitHub which...
@tomberek Should I add myself as maintainer? Is that the normal thing to do? |
Yes. Please make it an independent commit before the current one(same PR is fine). |
meta = with lib; { | ||
homepage = "https://github.com/c00kiemon5ter/iim"; | ||
license = licenses.mit; | ||
description = "iim is as a complete rewrite of the original ii from suckless.org"; |
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.
Adding yourself as a maintainer is required.
|
||
buildPhase = "make iim CFLAGS='-D_GNU_SOURCE -std=c99 -Os -Wall -Wextra -pedantic'"; | ||
|
||
installPhase = "install -D iim $out/bin/iim"; |
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.
installPhase = "install -D iim $out/bin/iim"; | |
installPhase = '' | |
install -D iim $out/bin/iim | |
''; |
rev = "220958e4e00156cbf67bf3cb4d60af25094f5428"; | ||
}; | ||
|
||
buildPhase = "make iim CFLAGS='-D_GNU_SOURCE -std=c99 -Os -Wall -Wextra -pedantic'"; |
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 use makeFlags.
stdenv.mkDerivation { | ||
name = "iim"; | ||
|
||
src = builtins.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.
You should use fetchFromGitHub which...
{ lib, stdenv }: | ||
stdenv.mkDerivation { |
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.
{ lib, stdenv }: | |
stdenv.mkDerivation { | |
{ lib, stdenv, fetchFromGitHub }: | |
stdenv.mkDerivation { |
.. would be added here.
@@ -0,0 +1,20 @@ | |||
{ lib, stdenv }: | |||
stdenv.mkDerivation { | |||
name = "iim"; |
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.
name = "iim"; | |
pname = "iim"; | |
version ="?"; |
Deleting this because multiple reviewers is annoying and @SuperSandro2000 doesn't know how to write complete sentences |
It is totally normal to get multiple reviewers if one does not have enough time to take another look at the PR. Normally there are also no contradicting requests/changes which shouldn't make this a problem. My sentence did start at the one comment and continued over the three dots ... into the other comment. I thought this was clear. |
Motivation for this change
I was trying to install iim and noticed that it wasn't in the package index, so decided that I'd add a package for it.
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)