diff mbox series

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

Message ID 6N2dsbDEMS8qfeyHPubfhcX_yqjbdQPCIY4d8TY_4eMJCBfWNMxNzOjiwR-6QhOyPw4s9s1uCGUNppmnaSuEbtR2HpQ3WX49EN1u7iVK2Ok=@protonmail.com
State Accepted
Headers show
Series [bug#37284] added gnu/packages/fmit.scm (Free Musical Instrument Tuner) | expand

Commit Message

ashish.is--- via Guix-patches" via Sept. 7, 2019, 10:12 p.m. UTC 
Hey!
Updated again. Is sending it as an attachment fine?

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, September 4, 2019 4:01 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> raingloom raingloom@protonmail.com 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

Comments

Nicolas Goaziou Sept. 8, 2019, 9:17 a.m. UTC | #1 
Hello,

raingloom <raingloom@protonmail.com> writes:

> Updated again. Is sending it as an attachment fine?

Well, oddly, it seems that your update addressed only one of my
suggestions. So, I applied your patch with the following changes:

- fixed the commit message,
- added a copyright line,
- added the (file-name ...) line,
- split the (sha256 ...) line,
- re-ordered the inputs,
- changed case in synopsis,
- fixed typographic mistakes in the description and the licenses
  comments.

Let me know if something is wrong, in particular the copyright line,
which I add to guess.

Thank you for your contribution!

Regards,
diff mbox series

Patch

From 3ddc9c6b62f61993f4c47c071a4c211997366eb3 Mon Sep 17 00:00:00 2001
From: raingloom <raingloom@protonmail.com>
Date: Mon, 2 Sep 2019 15:44:15 +0200
Subject: [PATCH] * gnu/packages/music.scm (fmit): New variable.

---
 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)))
+
+(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))))
+	      (sha256 (base32 "03nzkig5mw2rqwhwmg0qvc5cnk9bwh2wp13jh0mdrr935w0587mz"))))
+    (build-system gnu-build-system)
+    (arguments
+     '(#:phases
+       (modify-phases %standard-phases
+	 (delete 'configure)
+	 (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"))))
+         (add-after 'install 'wrap-executable
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let ((out (assoc-ref outputs "out")))
+               (wrap-program (string-append out "/bin/fmit")
+                 `("QT_PLUGIN_PATH" ":" prefix
+                   ,(map (lambda (label)
+                           (string-append (assoc-ref inputs label)
+                                          "/lib/qt5/plugins"))
+                         '("qtbase" "qtmultimedia" "qtsvg")))
+                 `("QML2_IMPORT_PATH" ":" prefix
+                   ,(map (lambda (label)
+                           (string-append (assoc-ref inputs label)
+                                          "/lib/qt5/qml"))
+                         '("qtmultimedia"))))
+               #t))))))
+    (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)))
+    (synopsis "Musical Instrument Tuner")
+    (description
+     "FMIT is a graphical utility for tuning musical instruments,
+with error and volume history, and advanced features")
+    (home-page "https://gillesdegottex.github.io/fmit/")
+    ;; most of the code is under GPL2+, but some abstract or helper classes are under LGPL2.1
+    (license (list license:gpl2+ license:lgpl2.1))))
-- 
2.23.0