-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
strawberry: init at 0.6.3 #67605
strawberry: init at 0.6.3 #67605
Conversation
@@ -0,0 +1,45 @@ | |||
{ mkDerivation, stdenv, lib, fetchFromGitHub, cmake, pkgconfig | |||
, alsaLib, boost, chromaprint, fftw, gnutls, libcdio, libmtp, libpthreadstubs, libtasn1, libXdmcp, pcre, protobuf, sqlite, taglib |
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.
This line is quite long, could it be split into two?
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 run nixpkgs-fmt on it to clean things up.
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 wasn't aware nixpkgs-fmt was released yet.
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 in unstable.
@@ -0,0 +1,45 @@ | |||
{ mkDerivation, stdenv, lib, fetchFromGitHub, cmake, pkgconfig |
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 never seen mkDerivation as a function arg but I guess it makes sense
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.
Though this does mean stdenv
has not been used anywhere
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.
See #65399.
stdenv is used in stdenv.isLinux
.
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.
of course, my bad
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.
Thanks for the link as well
}; | ||
|
||
buildInputs = [ | ||
alsaLib boost chromaprint fftw gnutls libcdio libmtp libpthreadstubs libtasn1 libXdmcp pcre protobuf sqlite taglib |
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.
this line is also quite long
qtbase qtx11extras | ||
] ++ lib.optionals stdenv.isLinux [ | ||
libpulseaudio libselinux libsepol p11_kit utillinux | ||
] ++ lib.optionals withVlc [ |
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.optional withVlc vlc
would work here
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.
Done.
I would run nix-review but it's trying to run a mass rebuild so I might try again when nixpkgs moves forwards |
This touches |
I gave nix-review another try but I think it'd take all day on my laptop. While running it I did get the following errors though they don't seem related to this package so I'm a bit confused.
Further, is it really correct that this package has 4 different versions of |
We should probably move that taglib change to staging instead. |
747f424
to
5a12e12
Compare
And here it is without the taglib change which is in its own PR. |
@GrahamcOfBorg build strawberry |
Motivation for this change
strawberry is a maintained fork of clementine. Can highly recommend it.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @ttuegel as clementine maintainer