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
imgproxy: init at 2.8.1 #77469
imgproxy: init at 2.8.1 #77469
Conversation
Welcome to nixpkgs! 😸 |
Thanks! :-) I've merged your fixes directly. Do we want to have this merge commit in history or should I squash it into single one and push? I've rerun |
Please squash it. |
No problem. Done :-) P.S. I'm not sure why github incorporates some additional commit in this PR... |
One sec. I've probably listed this additional commit during my rebase. Going to fix it... |
Cleaned up. Sorry for this additional spam :-) |
This looks great now, congrats! We'll now have to wait for a committer to merge this. |
pkgs/servers/imgproxy/default.nix
Outdated
{ lib, buildGoModule, fetchFromGitHub, pkg-config, vips, gobject-introspection }: | ||
|
||
buildGoModule rec { | ||
version = "2.8.1"; |
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.
Woops, forgot this one: Please move pname
above version
.
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.
Fwiw, I've stopped complaining about tiny issues like this, since I've been told there's too much bikeshedding like that, which can be demotivating for contributors.
pkgs/servers/imgproxy/default.nix
Outdated
pname = "imgproxy"; | ||
|
||
src = fetchFromGitHub { | ||
owner ="imgproxy"; |
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.
Add a space before "
Fixed and tested. @erikarvstedt thanks a lot for your time, review and support! |
pkgs/servers/imgproxy/default.nix
Outdated
export CGO_LDFLAGS_ALLOW='-(s|w)' | ||
''; | ||
|
||
goPackagePath = "github.com/imgproxy/imgproxy"; |
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.
goPackagePath
can be removed.
pkgs/servers/imgproxy/default.nix
Outdated
|
||
goPackagePath = "github.com/imgproxy/imgproxy"; | ||
|
||
modSha256 = "0kgd8lwcdns3skvd4bj4z85mq6hkk79mb0zzwky0wqxni8f73s6w"; |
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 put modSha256
below the src
definition.
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.
I've just dropped them and running test build on my machine. Should I bring them back?
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, my deleted comment was of course an error, I meant that goPackagePath
can be removed.
Sorry, my go-package-fu is a bit rusty, otherwise I would have included all these changes in my inital patch. |
pkgs/servers/imgproxy/default.nix
Outdated
version = "2.8.1"; | ||
|
||
src = fetchFromGitHub { | ||
owner = "imgproxy"; |
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.
owner = pname;
repo = pname;
No problem. I've removed |
Awesome, it's perfect now. Thanks for you patience! 😃 |
I've rerun test build. Everything is working fine.
I really appreciate your detailed review. Thanks again! |
@GrahamcOfBorg build imgproxy |
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.
Builds on NixOS. Functions as expected.
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.
Looks good to me
@erikarvstedt @stigtsp |
It's best to do it in a separate PR. |
Cool. I'm going to work on it along with the coding which I do for my internal project deployment. |
Motivation for this change
imgproxy server is not available in nixpkgs. I've packaged only module. I'm not sure if I should also try to provide appropriate service package (this is my first package).
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)