[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] gnu: Add lmms

From: julien lepiller
Subject: Re: [PATCH] gnu: Add lmms
Date: Thu, 23 Feb 2017 10:15:47 +0100
User-agent: Roundcube Webmail/1.2.3

Le 2017-02-23 00:18, Rodger Fox a écrit :
Here is another package. I think I got the commit message right this
time. Let me know if I missed anything.


I wanted to try this software for while now, so you're great :)

I added some comments on your patch below:

From 8e2757bee584f4686e02da512662fb512b05c037 Mon Sep 17 00:00:00 2001
From: Rodger Fox <address@hidden>
Date: Wed, 22 Feb 2017 15:08:30 -0800
Subject: [PATCH] gnu: Add lmms. * gnu/packages/music.scm (lmms): New variable.
The message is correct, but it lacks returns between "Add lmms." and "* gnu/packages". Have a look at git log for some examples.

+       (sha256
+        (base32
+        "1g76z7ha3hd53vbqaq9n1qg6s3lw8zzaw51iny6y2bz0j1xqwcsr"))))
There's a tab in the indentation, please use spaces.

+    (build-system cmake-build-system)
+    (arguments `(#:tests? #f              ; No tests to run.
+                #:validate-runpath? #f))
There's a tab here too, and it should rather look like this:

 `(#:tests? #f
   #:validate-runpath? #f))

Why do you need to disable runpath validation?

+    (native-inputs
+     `(("pkg-config" ,pkg-config)))
+     (inputs
Indentation is off by one (inputs should be aligned with native-inputs).

+ (description "LMMS is a digital audio workstation. It includes tools for sequencing melodies and beats and for mixing and arranging songs. It includes instruments based on audio samples and various soft sythesizers. It can receive input from a MIDI keyboard.")
This line is way too long, please break it. Also please use two spaces between sentences.

+    (license license:gpl2+)))

Ok, this is my first review, I tried to get it right but I probably forgot something (I still can't get my own patches right on the first try :p). Running "guix lint lmms" would have saved you the last comment ;). I can't try it now, but I'll test your patch (or an updated version) this evening.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]