bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Add (guix gnu-maintenance).


From: Ludovic Courtès
Subject: Re: [PATCH] Add (guix gnu-maintenance).
Date: Tue, 04 Dec 2012 19:42:02 +0100
User-agent: Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux)

Hi,

Nikita Karetnikov <address@hidden> skribis:

> +(define (http-fetch str)
> +  "Fetch a file from STR, which should be a URI."

Rather:

  (define (http-fetch uri)
    "Return a string containing the textual data at URI, a string."
    ...)

> +       (error "download failed:" str code
> +              (string-downcase (response-reason-phrase resp)))))))

Please remove ‘string-downcase’.

> +(define %womb

Rather ‘%package-list-url’.

> +  (string-append "http://cvs.savannah.gnu.org/";
> +                 "viewvc/*checkout*/gnumaint/"
> +                 "gnupackages.txt?root=womb"))
> +
> +(define-public (gnu-packages-womb)

What about ‘official-gnu-packages’ instead?  Seems more descriptive to me.

Also, please use #:export instead of ‘define-public’ (I try to avoid the
latter, and use it only in (distro packages ...) where it matters to
increase locality.)

> +  "Return a list of GNU packages."

  "Return the list of system identifiers of official GNU packages."

> +  (let ((lst (string-split (http-fetch %womb) #\nl)))
> +    (map (cut string-drop <> 9)
> +         (rx-filter lst (make-regexp "^package: (.+)$")))))

Rather:

  (define %package-line-rx
    (make-regexp "^package: (.+)$"))

  (let ((lst (string-split (http-fetch %package-list-url) #\nl)))
    (filter-map (lambda (line)
                  (and=> (regexp-exec %package-line-rx line)
                         (cut match:substring <> 1)))
                lst)

Can you post an updated patch?

Thanks,
Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]