On Sun, Apr 01, 2018 at 08:45:02PM +0530, Pierre Neidhardt wrote:
> * gnu/packages/video.scm (subdl): New variable.

Thanks!

> +(define-public subdl
> +  (let
> +      ((commit "4cf5789b11f0ff3f863b704b336190bf968cd471")
> +       (revision "1"))

It's okay to put 'commit' on the same line as 'let' :)

> +    (package
> +      (name "subdl")
> +      (version (string-append "1.0.3-" revision "." (string-take commit 7)))

Here we can use the git-version procedure and forget about how many
characters of the Git hash we are supposed to use:

(version (git-version "1.0.3" revision commit))

I was curious where the "1.0.3" comes from, since the upstream repo
lacks any version tags, but I found it in the subdl source code.

> +      (source (origin
> +                (method git-fetch)

And likewise in here, one should use git-file-name so that the source
archive is named descriptively:

(file-name (git-file-name name version))

> +      (build-system trivial-build-system)
> +      (arguments
> +       `(#:modules ((guix build utils))
> +         #:builder (begin
> +                     (use-modules (guix build utils))
> +                     (let* ((out (assoc-ref %outputs "out"))
> +                            (bin (string-append out "/bin"))
> +                            (source (assoc-ref %build-inputs "source"))
> +                            (python (assoc-ref %build-inputs "python")))
> +                       (mkdir-p bin)
> +                       (with-directory-excursion bin
> +                         (copy-file (string-append source "/subdl") "subdl")
> +                         (patch-shebang "subdl"
> +                                        (list (string-append python "/bin")))
> +                         (chmod "subdl" #o555))))))

You asked about this chmod in your followup message. In this case it's
unnecessary since the file is installed #o555 regardless. And we can
even simplify the code to this:

(install-file (string-append source "/subdl") bin) 
(patch-shebang (string-append bin "/subdl")
               (list (string-append python "/bin")))

The (install-file) procedure conveniently combines (mkdir-p) and
(copy-file).

I can commit with the changes in the attached diff, but which name and
email should I use for your copyright statement?