[bug#44181] [PATCH] gnu: Add atomicparsley.

[Christopher Baines <mail@cbaines.net>]

[-- Attachment #1: Type: text/plain, Size: 3307 bytes --]


Hi!

Thanks for the patch, I've put some comments inline below.

Alexandru-Sergiu Marton <brown121407@posteo.ro> writes:

> * gnu/packages/video.scm (atomicparsley): New variable.
> ---
>  gnu/packages/video.scm | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index bdccb6fdb7..5cf8b56ae1 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -44,6 +44,7 @@
>  ;;; Copyright © 2020 Michael Rohleder <mike@rohleder.de>
>  ;;; Copyright © 2020 Vinicius Monego <monego@posteo.net>
>  ;;; Copyright © 2020 Brett Gilio <brettg@gnu.org>
> +;;; Copyright © 2020 Alexandru-Sergiu Marton <brown121407@posteo.ro>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -4806,3 +4807,41 @@ BBC iPlayer output.")
>  includes @code{dvdxchap} tool for extracting chapter information from DVD.")
>      (license license:gpl2)
>      (home-page "https://www.bunkus.org/videotools/ogmtools/")))

Looks like this is at the end of video.scm. That's OK, but often unwise
as if everyone adds new packages to the end of the modules, the patches
will often conflict. For that reason, it's normally helpful to add
packages not at the end, I usually add it close to related packages, or
those with a name close by in the alphabet.

> +
> +(define-public atomicparsley
> +  (package
> +    (name "atomicparsley")
> +    (version "20200701.154658.b0d6223")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/wez/atomicparsley")
> +                    (commit version)))
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "1kym2l5y34nmbrrlkfmxsf1cwrvch64kb34jp0hpa0b89idbhwqh"))))
> +    (build-system cmake-build-system)
> +    (arguments
> +     `(#:tests? #f

So that someone looking at the package definition can tell why the tests
are disabled, it would be good to add a comment saying why. If it's that
the tests don't pass, then it can be a TODO comment.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'set-cmake-version
> +           (lambda* _
> +             (substitute* "CMakeLists.txt"
> +               (("VERSION 3.17") "VERSION 3.16"))
> +             #t))
> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (bin (string-append out "/bin")))
> +               (install-file "AtomicParsley" bin))
> +             #t)))))
> +    (inputs
> +     `(("zlib" ,zlib)))
> +    (synopsis "Metadata editor for MPEG-4 files")
> +    (description "AtomicParsley is a lightweight command line program for
> +reading, parsing and setting metadata into MPEG-4 files, in particular,
> +iTunes-style metadata.")
> +    (home-page "https://github.com/wez/atomicparsley")
> +    (license license:gpl2)))

I had a look at the few source files, and they suggested that this is
gpl2+, maybe there's a bit that's gpl2 only though?

Would you be able to send an updated patch?

Thanks,

Chris


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]