libayatana-appindicator: init at 0.5.4 libayatana-indicator: init at 0.6.3 ayatana-ido: init at 0.4.90 #88783
Conversation
| @@ -0,0 +1,49 @@ | |||
| { stdenv, fetchFromGitHub, lib | |||
| , pkgconfig, autoreconfHook , gtk-doc | |||
| , gtkVersion ? "3" | |||
There was a problem hiding this comment.
Typically this is done with something like withGtk3 ? true -- if withGtk3 then gtk3 else gtk2 and similar logic. This prevents the need to check gtkVersion == "3" and gtkVersion == "2", and also handles the case of somebody specifying gtkVersion = "1" (because there's no way to do that with withGtk3 -- it's either 2 or 3).
Same with the other files.
There was a problem hiding this comment.
I wasn't aware that these variables have conventions, I just followed what libappindicator did to be consistent; it makes sense for them both to match no, seeing as they are compatible libraries?
There was a problem hiding this comment.
You do what you think is best, but I would personally prefer the proposed method (and don't see the fact that it's compatible as a reason to carry around cruft in a new package).
| @@ -12573,6 +12575,10 @@ in | |||
| libappindicator-gtk3 = libappindicator.override { gtkVersion = "3"; }; | |||
| libappindicator = callPackage ../development/libraries/libappindicator { }; | |||
|
|
|||
| libayatana-appindicator-gtk2 = libayatana-appindicator.override { gtkVersion = "2"; }; | |||
| libayatana-appindicator-gtk3 = libayatana-appindicator.override { gtkVersion = "3"; }; | |||
| libayatana-appindicator = callPackage ../development/libraries/libayatana-appindicator { }; | |||
There was a problem hiding this comment.
Nit: I think it would make more sense to move the "base" package above the others (despite libappindicator not doing that just a few lines above).
| @@ -13008,6 +13014,10 @@ in | |||
| libindicator-gtk3 = libindicator.override { gtkVersion = "3"; }; | |||
| libindicator = callPackage ../development/libraries/libindicator { }; | |||
|
|
|||
| libayatana-indicator-gtk2 = libayatana-indicator.override { gtkVersion = "2"; }; | |||
| libayatana-indicator-gtk3 = libayatana-indicator.override { gtkVersion = "3"; }; | |||
| libayatana-indicator = callPackage ../development/libraries/libayatana-indicator { }; | |||
There was a problem hiding this comment.
Same as above: please move the "base" package above the overrides.
|
@cole-h Let's get those in a separate PR. It's more useful to match the interface as this PR does for a smooth migration, even if that means miming dubious things, and then clean up both interfaces after. We could commute the steps too: clean up the old library's interface, and then add the new library matching the now--improved interface. That's not necessary better or worse---the important thing being the smooth transition. But given that this PR already exists, I'd say that tips the scales so it's land this then cleanup. |
| description = "Ayatana Application Indicators Shared Library"; | ||
| homepage = "https://github.com/AyatanaIndicators/libayatana-appindicator"; | ||
| changelog = "https://github.com/AyatanaIndicators/libayatana-appindicator/blob/${version}/ChangeLog"; | ||
| license = [ licenses.gpl3 licenses.lgpl21 ]; |
There was a problem hiding this comment.
gpl3 is unclear, use gpl3Plus or gpl3Only. See https://discourse.nixos.org/t/lib-licenses-gpl3-co-are-now-deprecated/8206 for more details.
| --replace "codegendir pygtk-2.0" "codegendir pygobject-2.0" | ||
| ''; | ||
|
|
||
| nativeBuildInputs = [ pkgconfig autoreconfHook gtk-doc ]; |
There was a problem hiding this comment.
In new expressions, pkg-config should be used instead of the pkgconfig alias.
|
|
||
| nativeBuildInputs = [ pkgconfig autoreconfHook gtk-doc ]; | ||
|
|
||
| buildInputs = [ dbus-glib python2 python2Packages.pygtk ] |
| changelog = "https://github.com/AyatanaIndicators/libayatana-appindicator/blob/${version}/ChangeLog"; | ||
| license = [ licenses.gpl3 licenses.lgpl21 ]; | ||
| maintainers = [ maintainers.nickhu ]; | ||
| platforms = platforms.x86_64; |
| sha256 = "0bqjqb7gabdk7mifk8azi630qw39z978f973fx2ylgdgr4a66j1v"; | ||
| }; | ||
|
|
||
| patchPhase = '' |
There was a problem hiding this comment.
Overriding patchPhase breaks patches field. Use prePatch or postPatch instead.
Motivation for this change
Adds the libayatana-appindicator library, which supersedes the
deprecated libappindicator library.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)