[bug#37284] [PATCH] added gnu/packages/fmit.scm (Free Musical Instrument Tuner)

[Nicolas Goaziou]

Hello,

raingloom <address@hidden> writes:

> Thanks for the suggestions!
> How about this attempt?

Better, indeed. I have a couple more comments, if you don't mind.
I think I will be able to commit it at next iteration.

> Also, is this how i reply to an issue? 

Like you did.

> Do I also have to subscribe to the guix-patches mailing list?

I don't think so.

> ---
>  gnu/packages/music.scm | 59 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
>
> diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
> index d4aaecc1e2..a9770f4f16 100644
> --- a/gnu/packages/music.scm
> +++ b/gnu/packages/music.scm
> @@ -4483,3 +4483,62 @@ discard bad quality ones.
>  controller.")
>      (home-page "https://github.com/charlesfleche/lpd8editor";)
>      (license license:expat)))

Could you add a proper commit message to the patch ? It would look like:

* gnu/packages/music.scm (fmit): New variable.

Also, could you add yourself to the copyright line at the top of the
"music.scm" file?

> +(define-public fmit
> +  (package
> +    (name "fmit")
> +    (version "1.2.6")
> +    (source (origin
> +           (method git-fetch)
> +           (uri (git-reference
> +                 (url "https://github.com/gillesdegottex/fmit/";)
> +                 (commit (string-append "v" version))))

You need to add the following line here:

    (file-name (git-file-name name version))

> +           (sha256 (base32 
> "03nzkig5mw2rqwhwmg0qvc5cnk9bwh2wp13jh0mdrr935w0587mz"))))

This line is too long, you ought to use

    (sha256 
     (base32
      "..."))

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

You forgot to re-order inputs alphabetically.

> +    (synopsis "Musical Instrument Tuner")

I don't think title case is warranted here. "Musical instrument tuner"
is enough, IMO.

> +    (description
> +     "FMIT is a graphical utility for tuning musical instruments,
> +with error and volume history, and advanced features")

Please move the string so that it starts on the same line as
"(description ".  Also, it is missing a final dot.

> +    (home-page "https://gillesdegottex.github.io/fmit/";)
> +    ;; most of the code is under GPL2+, but some abstract or helper
> classes are under LGPL2.1

  ;; Most of ... under LGPL2.1.

i.e., missing capital and the final full stop.

Could you send an updated patch? Thank you!

Regards,

-- 
Nicolas Goaziou