From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: swh-plugins-lv2: New variable [WIP] v2 Date: Mon, 07 Dec 2015 22:17:38 +0100 Message-ID: <87wpsqm00d.fsf@elephly.net> References: <5664BA45.3080600@gmx.net> <5664BB8C.6030100@gmx.net> 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]:47809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a63AL-0004Bz-US for guix-devel@gnu.org; Mon, 07 Dec 2015 16:17:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a63AI-0006w4-M4 for guix-devel@gnu.org; Mon, 07 Dec 2015 16:17:49 -0500 Received: from sender163-mail.zoho.com ([74.201.84.163]:25426) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a63AI-0006vu-EP for guix-devel@gnu.org; Mon, 07 Dec 2015 16:17:46 -0500 In-reply-to: <5664BB8C.6030100@gmx.net> 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-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Florian Paul Schmidt Cc: guix-devel@gnu.org Hi Florian, thanks for the patch! > From 6dee84494a522921baacbcad8c7618c9eb709eb1 Mon Sep 17 00:00:00 2001 > From: Florian Paul Schmidt > Date: Wed, 2 Dec 2015 15:30:14 +0100 > Subject: [PATCH] swh-plugins-lv2: New variable The commit message should be: gnu: Add swh-plugins-lv2. * gnu/packages/audio.scm (swh-plugins-lv2): New variable. > --- > gnu/packages/audio.scm | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm > index db3f912..ec91c33 100644 > --- a/gnu/packages/audio.scm > +++ b/gnu/packages/audio.scm > @@ -1891,3 +1891,43 @@ access to ALSA PCM devices, taking care of the many functions required to > open, initialise and use a hw: device in mmap mode, and providing floating > point audio data.") > (license license:gpl3+))) > + > +(define-public swh-plugins-lv2 > + (let ((commit "5098e09e255eaed14e0d40ca5e7e6dfcb782d7ea")) We usually don’t use full commit hashes. You could probably trim it to the first six characters or so. It would also be nice to state in a comment why you used this commit (e.g. because that’s the latest commit as of now, and there haven’t been any releases). > + (package > + (name "swh-plugins-lv2") > + (version (string-append "2015-11-11-" commit)) > + (source (origin > + (method url-fetch) > + (uri (string-append "https://github.com/swh/" > + "lv2/archive/" > + commit ".zip")) There is no good reason to use “.zip” here. “.tar.gz” will work just fine and you won’t need the “unzip” input then. > + (sha256 > + (base32 > + "11c6y4nfw5kz7647axpgdaryiiwwrplnkdbkglx362cbcmhpvds8")))) > + (build-system gnu-build-system) > + (arguments > + `(#:phases > + (alist-cons-after Please use ‘modify-phases’ instead of the ‘alist-*’ stuff. > + 'unpack 'patch-makefile-and-enter-directory > + (lambda > + _ This should not be on its own line. > + (substitute* "Makefile" > + (("/usr/local") (assoc-ref %outputs "out")) I don’t think this is really necessary. You could just add a definition for “PREFIX” to the make-flags. > + (("install:") "install: install-system"))) > + (alist-delete > + 'check Use “#:tests? #f” instead with a note why tests are disabled. > + (alist-delete > + 'configure Also here please add a note why. > + %standard-phases))) > + #:make-flags '("CC=gcc"))) > + (inputs > + `(("lv2" ,lv2) > + ("unzip" ,unzip) You don’t need “unzip” (see above), but had you actually needed it you should have added it to “native-inputs”. > + ("fftw" ,fftw) > + ("libxslt" ,libxslt))) > + (home-page "http://plugin.org.uk") > + (synopsis "SWH Plugins in LV2 format") > + (description > + "A collection of Steve Harris' audio plugins in LV2 format.") “guix lint” probably complains about this description, because it is a sentence fragment, not a full sentence. It would also be nice if this would include a list of plugin classes that are among this collection. People who are looking for a chorus effect with “guix package -s chorus”, for example, would not find this package. It doesn’t have to be the complete list of plugins, but the categories that are covered should be mentioned (e.g. “filters” instead of “lowpass, butterworth, simple comb, ...”). > + (license license:gpl3+)))) Could you please send an updated patch (after running your final version through “guix lint”)? Thanks! ~~ Ricardo