[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#52113] [PATCH] gnu: Add pnmixer
From: |
Nicolas Goaziou |
Subject: |
[bug#52113] [PATCH] gnu: Add pnmixer |
Date: |
Sun, 05 Dec 2021 21:43:20 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Hello,
Jaft <jaft.r@outlook.com> writes:
> * gnu/packages/gtk.scm (pnmixer):Add PNMixer
Thank you. Some comments follow.
> +(define-public pnmixer
> + (let ([version "0.7.2"])
Nitpick: we don't use square brackets for let. Besides, you do not need
a binding here, you just need to hard-code it in the version field.
> + (package
> + (name "pnmixer")
> + (version version)
> + (source (origin
Could you move origin below source?
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/nicklan/pnmixer/")
> + (commit (string-append "v" version))))
> + (file-name (git-file-name name version))
> + (sha256 (base32
> +
> "0416pa933ddf4b7ph9zxhk5jppkk7ppcq1aqph6xsrfnka4yb148"))
Could you move base32 below sha256 and put the hash string in front of base32?
> + (modules '((guix build utils)))))
The modules part is not required. You can remove it.
> + (build-system cmake-build-system)
> + (arguments `(#:phases (modify-phases %standard-phases (delete 'check))))
The correct way to do this is to add a "#:tests? #f" argument, with
a comment explaining why you are disabling tests.
> + (native-inputs `(("pkg-config" ,pkg-config)
> + ("gettext" ,gettext-minimal)))
Native inputs should be ordered alphabetically. Besides, the list should
be moved on the line below native-inputs.
> + (inputs `(("alsa-lib" ,alsa-lib)
> + ("glib" ,glib)
> + ("libx11" ,libx11)
> + ("gtk+" ,gtk+)
> + ("libnotify" ,libnotify)))
Ditto: please order inputs alphabetically and move them below the inputs
field name.
> + (home-page "https://github.com/nicklan/pnmixer/")
> + (synopsis "Simple mixer application designed to run in your system tray")
Nitpick: you can remove "your" in the synopsis
Could you send an updated patch?
Regards,
--
Nicolas Goaziou
- [bug#52113] [PATCH] gnu: Add pnmixer,
Nicolas Goaziou <=