From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH v2] gnu: Add cbatticon. Date: Tue, 09 Aug 2016 21:06:29 +0200 Message-ID: <87eg5xep8q.fsf@elephly.net> References: <87y44528hf.fsf@we.make.ritual.n0.is> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:36952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXCML-0003J0-Dd for guix-devel@gnu.org; Tue, 09 Aug 2016 15:06:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXCMH-0003Ff-2r for guix-devel@gnu.org; Tue, 09 Aug 2016 15:06:40 -0400 Received: from sender163-mail.zoho.com ([74.201.84.163]:24988) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXCMG-0003FS-QV for guix-devel@gnu.org; Tue, 09 Aug 2016 15:06:37 -0400 In-reply-to: <87y44528hf.fsf@we.make.ritual.n0.is> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: ng0 Cc: guix-devel@gnu.org ng0 writes: > From ac578d27529cc2a5f39f66054b5991e44e65f0b9 Mon Sep 17 00:00:00 2001 > From: ng0 > Date: Tue, 9 Aug 2016 16:47:37 +0000 > Subject: [PATCH] gnu: Add cbatticon. > * gnu/packages/admin.scm (cbatticon): New variable. > --- > gnu/packages/admin.scm | 46 +++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) > diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm > index eada796..4025300 100644 > --- a/gnu/packages/admin.scm > +++ b/gnu/packages/admin.scm > @@ -12,6 +12,7 @@ > ;;; Copyright © 2016 Efraim Flashner > ;;; Copyright © 2016 Peter Feigl > ;;; Copyright © 2016 John J. Foerch > +;;; Coypright © 2016 ng0 > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -70,7 +71,9 @@ > #:use-module (gnu packages xorg) > #:use-module (gnu packages python) > #:use-module (gnu packages man) > - #:use-module (gnu packages autotools)) > + #:use-module (gnu packages autotools) > + #:use-module (gnu packages gnome) > + #:use-module (gnu packages gtk)) > (define-public aide > (package > @@ -1698,3 +1701,44 @@ a new command using the matched rule, and runs it.") > display your disk usage in whatever format you prefer. It is designed to be > highly portable. Great for heterogenous networks.") > (license license:zlib))) > + > +(define-public cbatticon > + (package > + (name "cbatticon") > + (version "1.6.4") > + (source (origin > + (method url-fetch) > + (uri (string-append "https://github.com/valr/"; > + name "/archive/" version ".tar.gz")) > + (sha256 > + (base32 > + "023fvsa4q7rl98rqgwrb1shyzaybdkkbyz5sywd0s5p7ixkksxqx")) > + (file-name (string-append name "-" version ".tar.gz")))) > + (build-system gnu-build-system) > + (arguments > + `(#:tests? #f ; no make check Nit-pick: I’d write “no "check" target” or “no tests”. > + #:make-flags > + (list (string-append "PREFIX=" (assoc-ref %outputs "out")) > + "CC=gcc") > + #:phases > + (modify-phases %standard-phases > + (delete 'configure) ; no configure script > + (add-before 'build 'patch-paths-in-Makefile > + (lambda* (#:key outputs #:allow-other-keys) > + (lambda _ Why is this a lambda inside of another lambda? This means that the substitution really doesn’t happen at build time. This build phase only returns a function and then moves on. > + (substitute* "Makefile" > + (("msgfmt") (which "msgfmt")) > + (("RM = rm -f") > + (string-append "RM = " (which "rm") " -f"))))))))) These substitutions don’t seem necessary to me. (Considering that this doesn’t get executed due to the nested lambda, maybe this is really not needed.) > + (propagated-inputs > + `(("libnotify" ,libnotify))) Why is this propagated? This shouldn’t be needed. Propagation is best avoided. > + (inputs > + `(("gtk+" ,gtk+) > + ("gnu-gettext" ,gnu-gettext))) > + (native-inputs > + `(("pkg-config" ,pkg-config))) > + (synopsis "A lightweigth battery icon for the system tray") Please don’t begin the synopsis with an article (“A”). s/lightweigth/lightweight/ > + (description "cbatticon is a lightweight battery icon that displays > +the status of your battery.") How about adding “in the system tray” to the end of the sentence? > + (home-page "https://github.com/valr/cbatticon") > + (license license:gpl2+))) Okay. Could you please send an updated patch? Thanks! ~~ Ricardo