Subject: Re: bug#25741: [PATCH 1/2] gnu: Add kbd-neo.

From: Ricardo Wurmus <[email-masked]>
Date: Thu, 09 Mar 2017 08:07:56 +0100

[email-masked] writes:

> From: ng0 <[email-masked]>
>
> * gnu/packages/linux.scm (kbd-neo): New variable.
[…]
> +;; The Neo layout is a GPL3 licensed layout already present in Xorg,
> +;; but not in kbd. home-page: https://neo-layout.org

Please remove this comment.

> +(define kbd-neo
> +  (let ((svn-revision 2476)
> +        (revision "1"))

SVN revisions are monotonically increasing, so we don’t need the
“revision” hack.

> +    (package
> +      (name "kbd-neo")
> +      (version (string-append "0.0.0-" revision "."
> +                              (number->string svn-revision)))
> +      (source
> +       (origin
> +         (method svn-fetch)
> +         (uri (svn-reference
> +               ;; The SVN branch with the neo.map file.
> +               (url "https://svn.neo-layout.org/linux/console")
> +               (revision svn-revision)))
> +         (file-name (string-append name "-" version "-checkout"))
> +         (sha256
> +          (base32
> +           "169hmlwcwz5sp605i8q5ikckxwsj1n3isylrnynflp30gbv9zrwn"))))

Actually, you can get the neo.map file at the given revision 2476 with
“url-fetch” from this URL:

    https://svn.neo-layout.org/!svn/bc/2476/linux/console/neo.map

This would be better than to use the more complicated “svn-fetch”.

> +      (build-system trivial-build-system)
> +      (arguments
> +       `(#:modules ((guix build utils))
> +         #:builder (begin
> +                     (use-modules (guix build utils))
> +                     (let ((out (string-append %output
> +                                               "/share/keymaps/i386/neo"))
> +                           (source (assoc-ref %build-inputs "source")))
> +                       (mkdir-p out)
> +                       (copy-file (string-append source "/neo.map")
> +                                  (string-append out "/neo.map"))))))

If you’re using the plain URL above this would be simplified a little.

> +      (native-inputs
> +       `(("source" ,source)))

I don’t think this is needed

> +      (home-page "https://neo-layout.org")
> +      (license license:gpl3)

Please move the license field below “description” for consistency.  It
is unfortunate that there are no license headers and that the license in
the COPYING file is explicitly declared to apply to all files in the
repository.

> +      (synopsis "Neo2 console font")

This is not a font as far as I can see.

> +      (description
> +       "Neo2 console font intended to be installed with kbd."))))
> +

Please use full sentences in the description.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net