GNU bug report logs - #30009
[PATCH 0/1] gnu: Add selene.

Previous Next

Full log

Message #11 received at 30009 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Fis Trivial <ybbs.daans <at> hotmail.com>
Cc: "30009 <at> debbugs.gnu.org" <30009 <at> debbugs.gnu.org>
Subject: Re: [bug#30009] [PATCH 1/1] gnu: Add selene.
Date: Mon, 08 Jan 2018 09:58:14 +0100

Hello,

Some mostly cosmetic suggestions, which hopefully answer your questions:

Fis Trivial <ybbs.daans <at> hotmail.com> skribis:

> * gnu/packages/lua.scm (selene): New public variable.

[...]

> +         (replace 'install
> +           (lambda _
> +             (let* ((output     (assoc-ref %outputs "out"))
> +                    (source     (assoc-ref %build-inputs "source"))
> +                    (includedir (string-append output "/include")))

Avoid the ‘%outputs’ and ‘%build-inputs’ global variables by writing:

  (lambda* (#:key inputs outputs #:allow-other-keys)
    (let ((output (assoc-ref outputs "out")))
      …))

> +         ;; The path of test files are hard coded.
> +         (replace 'check
> +           (lambda _

Likewise.

> +             (let* ((output (assoc-ref %output "out"))
> +                    (source (assoc-ref %build-inputs "source"))
> +                    (builddir   (getcwd))
> +                    (testdir    (string-append builddir "/test")))
> +               (and
> +                (copy-recursively
> +                 (string-append source "/test")
> +                 testdir)
> +                (system* "make")
> +                ;; To overcome the hardcoded test path
> +                (mkdir-p "runner")
> +                (copy-file "./test_runner" "./runner/test_runner")
> +                (chdir "./runner")
> +                (system* "./test_runner"))))))))

Here you can remove ‘and’ and use ‘invoke’ instead of ‘system*’
(‘invoke’ throws an exception when execution fails.)

> +    (home-page "https://github.com/jeremyong/Selene")
> +    (synopsis "Lua C++11 bindings")
> +    (description
> +     "Selene is a simple C++11 friendly header-only binding to Lua.")

It’s a library to create Lua bindings, pretty much like Boost::Python,
isn’t it?  Perhaps the description could clarify that somehow.

Otherwise LGTM.  Could you send an updated patch?

Thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.