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
Submit/stagit gopher #90502
Submit/stagit gopher #90502
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.
Few minor nits, otherwise LGTM
Indentation is off, should be two spaces i think.
Commits don't follow the contributing guidelines (should be squashed, with the message as described in there). Should note that the maintainer list addition should be in a seperate commit as well.
{ stdenv, libgit2, fetchgit }: | ||
|
||
stdenv.mkDerivation rec { | ||
name = "stagit-gopher"; |
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 be using pname
here.
makeFlags = [ "PREFIX=$(out)" ]; | ||
|
||
buildInputs = [ libgit2 ]; | ||
|
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.
Second newline isn't needed
OK @IvarWithoutBones , So I should fix the formatting in the actual code files, squash the commits and do a separate pull request for the maintainers file? |
Yep, except that the maintainer file changes can be in this PR, it should just be in a different commit than the actual package addition. |
798c7ab
to
ee0a570
Compare
@IvarWithoutBones, Right think I've got it this time. Maintainer and package in in separate commits and I think I've got the formatting sorted this time. Sorry about the slight ineptitude with getting the commits correctly squashed together. Must have taken about three or four resets |
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.
Sorry about the slight ineptitude with getting the commits correctly squashed together. Must have taken about three or four resets
No worries, git can be a bit confusing sometimes
LGMT :)
Output from nixpkgs-review
:
1 package built:
stagit-gopher
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.
Apologies, looks like i missed a few more nits
url = "git://git.codemadness.org/stagit-gopher"; | ||
rev = version; | ||
sha256 = "0dcyi3k900jb8p5qhgfk91gvplblysgfz1vyn6pqrlk931hpac80"; | ||
}; |
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 seems to be wrong here
description = "Static git site generator for gopher"; | ||
license = licenses.mit; | ||
platforms = platforms.all; | ||
maintainers = with maintaininers; [ hcssmith ]; |
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.
Typo here, causing ofborg to fail.
maintainers = with maintaininers; [ hcssmith ]; | |
maintainers = with maintainers; [ hcssmith ]; |
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.
OK, I've corrected the typo, not sure how i missed that given that I wrote it two words before, and I have moved the } back two spaces which I think is what you meant.
ee0a570
to
e3a95ad
Compare
I marked this as stale due to inactivity. → More info |
stagit-gopher init at version 0.9.3
Motivation for this change
So I can use stagit-gopher on Nixos
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)