[bug#51920] [PATCH] gnu: Add inkbox.

[Nicolas Goaziou]

Hello,

phodina via Guix-patches via <guix-patches@gnu.org> writes:

> * gnu/packages/qt.scm (inkbox): New variable.

Thank you. Some comments follow.

First, this should go to "ebook.scm" instead of "qt.scm".

> +    (source (origin

You should instead a newline before "(origin".

> +              (method git-fetch)
> +              (uri
> +               (git-reference
> +                (url "https://alpinekobox.ddns.net/InkBox/inkbox/";)
> +                (commit version)))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "126cqn0ixcn608lv2hd9f7zmzj4g448bnpxc7wv9cvg83qqajh5n"))))

Nitpick: hash should go on the same line as base32.

> +    (build-system qt-build-system)
> +    (arguments
> +     '(#:tests? #f

You should add a comment explaining why tests are disabled (i.e.: there
is no tests).

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'prefix-opt
> +           (lambda* _
> +             (substitute* "inkbox.pro"
> +               (("/opt/\\$\\$\\{TARGET\\}") (string-append (assoc-ref 
> %outputs "out"))))))

You should use (lambda* (#:key outputs #:allow-other-keys) ...) and then
(assoc-ref outputs "out") instead.

> +         (replace 'configure
> +           (lambda* (#:key make-flags #:allow-other-keys)
> +             (apply invoke (cons "qmake" make-flags)))))))
> +    (native-inputs
> +     `(("qtbase" ,qtbase-5)))

Does it fail with qtbase 6?

> +    (license license:gpl1)))

License is actually gpl3, according to LICENSE file.

Could you send an updated patch?

Regards,
-- 
Nicolas Goaziou