From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kost Subject: Re: [PATCH 1/5] import: Add 'elpa' importer Date: Sun, 21 Jun 2015 23:39:42 +0300 Message-ID: <87wpyw6bjl.fsf@gmail.com> References: Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:40162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z6m1v-0004Sq-A0 for guix-devel@gnu.org; Sun, 21 Jun 2015 16:39:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z6m1q-0000Ao-GB for guix-devel@gnu.org; Sun, 21 Jun 2015 16:39:51 -0400 Received: from mail-la0-x22b.google.com ([2a00:1450:4010:c03::22b]:33381) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z6m1q-0000AB-8e for guix-devel@gnu.org; Sun, 21 Jun 2015 16:39:46 -0400 Received: by laka10 with SMTP id a10so99167109lak.0 for ; Sun, 21 Jun 2015 13:39:45 -0700 (PDT) In-Reply-To: (Federico Beffa's message of "Sun, 21 Jun 2015 10:28:27 +0200") 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: Federico Beffa Cc: Guix-devel Hi, I've tried this elpa importer and it is great!! I don't have real comments on code, just some nitpicks. Federico Beffa (2015-06-21 11:28 +0300) wrote: > From 8796b32a1ff8d565c3eb9874cde6a4a14d0b4f3b Mon Sep 17 00:00:00 2001 > From: Federico Beffa > Date: Tue, 16 Jun 2015 10:50:06 +0200 > Subject: [PATCH 1/5] import: Add 'elpa' importer. > > * guix/import/elpa.scm: New file. > * guix/scripts/import.scm: Add "elpa" to 'importers'. > * guix/scripts/import/elpa.scm: New file. > * Makefile.am (MODULES): Add 'guix/import/elpa.scm' and > 'guix/scripts/import/elpa.scm'. (SCM_TESTS): Add 'tests/elpa.scm'. AFAIK the convention is to put "(...): ..." on a separate line: * Makefile.am (MODULES): Add 'guix/import/elpa.scm' and 'guix/scripts/import/elpa.scm'. (SCM_TESTS): Add 'tests/elpa.scm'. [...] > diff --git a/doc/guix.texi b/doc/guix.texi > index 46dccb8..3ca105a 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -3770,6 +3770,33 @@ package name by a hyphen and a version number as in the following example: > @example > guix import hackage mtl-2.1.3.1 > @end example > + > +@item elpa > +@cindex elpa > +Import meta-data from an Emacs ELPA package repository. > + > +Specific command-line options are: > + > +@table @code > +@item --archive=@var{repo} > +@itemx -a @var{repo} > +@var{repo} identifies the archive repository to from which to retrive Redundant "to" (before "from")? And a typo in "retrieve". [...] > +(define* (elpa-package->sexp pkg) There are some trailing spaces in this procedure: > + "Return the `package' S-expression for the Emacs package PKG, a record of > +type ''." > + here^ > + (define name (elpa-package-name pkg)) > + here^ > + (define version (elpa-package-version pkg)) > + here^ > + (define source-url (elpa-package-source-url pkg)) > + here^ > + (define dependencies > + (let* ((deps (elpa-package-inputs pkg)) > + (names (filter-dependencies (elpa-dependencies->names deps)))) > + (map (lambda (n) > + (let ((new-n (elpa-name->package-name n))) > + (list new-n (list 'unquote (string->symbol new-n))))) > + names))) > + here^ > + (define (maybe-inputs input-type inputs) > + (match inputs > + (() > + '()) > + ((inputs ...) > + (list (list input-type > + (list 'quasiquote inputs)))))) > + here^ [...] > +;;; Command-line options. > +;;; > + > +(define %default-options > + '((repo . "gnu"))) > + > +(define (show-help) > + (display (_ "Usage: guix import elpa PACKAGE-NAME > +Import the latest package named PACKAGE-NAME from an ELPA repository.\n")) > + (display (_ " > + -a ARCHIVE, --archive=ARCHIVE specify the archive repository")) I think we don't duplicate an option argument (see "guix package --help" for example): -a, --archive=ARCHIVE specify the archive repository -- Alex