unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: 37284@debbugs.gnu.org
Subject: [bug#37284] [PATCH] added gnu/packages/fmit.scm (Free Musical Instrument Tuner)
Date: Mon, 02 Sep 2019 19:29:34 +0200	[thread overview]
Message-ID: <87ef0yr4ht.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <UidibyLcn3xTwjDrxMaQPt6gOp2iz1d42z4V71_RpF1uRvC6-Vhn-1br5zn7snkgfzjsa4SB9UD2u-ZU7wboCHLchlXrtsbXAEFvkKjUayg=@protonmail.com> (raingloom via Guix-patches via's message of "Mon, 02 Sep 2019 16:16:19 +0000")

Hello,

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

> * gnu/packages/fmit.scm: created

Thank you!

> ---
>  gnu/packages/fmit.scm | 64 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 gnu/packages/fmit.scm
>
> diff --git a/gnu/packages/fmit.scm b/gnu/packages/fmit.scm

Is there any reason to create a new module instead of putting the
package into "music.scm"?

> +    (arguments
> +     '(#:make-flags (let ((out (assoc-ref %outputs "out"))) (list (string-append "PREFIX=" out) (string-append "PREFIXSHORTCUT=" out)))

This line is too long. Moreover, I think "PREFIX=" is not necessary, as
Guix adds it automatically. I'm not sure about "PREFIXSHORTCUT=", tho.

It seems PREFIX and PREFIXSHORTCUT are used in qmake, not in make, so it
may be possible to remove the whole #:make-flags altogether. WDYT?

> +       #:phases
> +       (modify-phases %standard-phases
> +	 (delete 'configure)

Indentation looks wrong. Could you take care about it?

> +	 (add-before 'build 'qmake
> +	   (lambda _
> + (let ((out (assoc-ref %outputs "out"))) (invoke "qmake" "fmit.pro"
> (string-append "PREFIX=" out) (string-append "PREFIXSHORTCUT=" out)
> "CONFIG+=acs_qt acs_alsa acs_jack acs_portaudio"))))

The line is too long. I suggest:

  (let ((out (assoc-ref %outputs "out")))
    (invoke "qmake"
            "fmit.pro"
            (string-append "PREFIX=" out)
            (string-append "PREFIXSHORTCUT=" out)
            "CONFIG+=acs_qt acs_alsa acs_jack acs_portaudio"))

> +    (inputs
> +     `(("fftw" ,fftw)
> +       ("portaudio" ,portaudio)
> +       ("qtmultimedia" ,qtmultimedia)
> +       ("qtsvg" ,qtsvg)
> +       ("alsa-lib" ,alsa-lib)
> +       ("jack" ,jack-1)
> +       ("qtbase" ,qtbase)))
> +    (native-inputs
> +     `(("itstool" ,itstool)
> +       ("qttools" ,qttools)
> +       ("hicolor-icon-theme" ,hicolor-icon-theme)
> +       ("gettext" ,gnu-gettext)))

Could you re-order (native-)inputs alphabetically?

> +    (synopsis "Free Musical Instrument Tuner")

I understand Free is the F from FMIT, but, as a synopsis, I don't think
it brings much value. Maybe "Musical instrument tuner" is enough?

> +    (description "FMIT is a graphical utility for tuning musical instruments, with error and volume history, and advanced features")
> +    (home-page "http://gillesdegottex.github.io/fmit/")

Prefer https:// here.

> +    (license gpl3+)))

If you move it to music.scm, I believe it should be prefixed with
license:

Also, the license seems wrong, it should be (list license:gpl2+
license:lgpl2.1) with a comment explaining that most of the code is
under GPL2+, but some abstract or helper classes are under LGPL2.1.

Regards,

-- 
Nicolas Goaziou

  reply	other threads:[~2019-09-02 17:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-02 16:16 [bug#37284] [PATCH] added gnu/packages/fmit.scm (Free Musical Instrument Tuner) raingloom via Guix-patches via
2019-09-02 17:29 ` Nicolas Goaziou [this message]
2019-09-03  1:13 ` raingloom via Guix-patches via
2019-09-04 16:01   ` Nicolas Goaziou
2019-09-07 22:12     ` raingloom via Guix-patches via
2019-09-08  9:17       ` bug#37284: " Nicolas Goaziou
2019-09-08  9:25         ` [bug#37284] " raingloom via Guix-patches via
     [not found] ` <handler.37284.D37284.156793426311682.notifdone@debbugs.gnu.org>
2019-09-08  9:22   ` [bug#37284] closed (Re: [bug#37284] [PATCH] added gnu/packages/fmit.scm (Free Musical Instrument Tuner)) raingloom via Guix-patches via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ef0yr4ht.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=37284@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).