guix-patches
[Top][All Lists]
Advanced

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

bug#25918: [PATCH 1/3] gnu: Add libmesode.


From: Catonano
Subject: bug#25918: [PATCH 1/3] gnu: Add libmesode.
Date: Tue, 14 Mar 2017 13:38:29 +0100

Mekeor,

sorry for the delay  in reviewing this patch. I'm afraid Ricardo forgot this. Patches are becoming to many and reviewers are not increasing that much, as far as I understand

But that's no problem, we can start reviewing on our own ;-)

2017-03-01 20:23 GMT+01:00 Mekeor Melire <address@hidden>:

+(define-public libmesode
+  (package
+    (name "libmesode")
+    (version "0.9.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/boothj5/libmesode/archive/"
+                                  version ".tar.gz")

I think there is a missing closing paren at the end of the "uri" field descriptor.
 
+              (sha256
+               (base32
+                "0iaj56fkd5bjvqpvq3324ni895rmbj1akbfqipjydnghfwaym4z6"))))

Here, you could use a line like this
                      (file-name (string-append name "-" version "-checkout"))

Otherwise the linter complains that this is missing. Like this

gnu/packages/messaging.scm:1230:12: address@hidden: the source file name should contain the package name

 
+    (build-system gnu-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-before 'configure 'bootstrap
+           (lambda _
+             (zero? (system* "./bootstrap.sh")))))))
+    (inputs
+     `(("expat" ,expat)
+       ("openssl" ,openssl)))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("automake" ,automake)
+       ("libtool" ,libtool)
+       ("pkg-config" ,pkg-config)))
+    (synopsis "A simple, lightweight C library for writing XMPP clients")

The linter complains that a synopsis shouldn't start with an article. Like this

gnu/packages/messaging.scm:1252:14: address@hidden: no article allowed at the beginning of the synopsis

 
+    (description "Libmesode is a fork of libstrophe for use with Profanity
+XMPP Client. In particular, libmesode provides extra TLS functionality such as

The linter complains that there should be 2 spaces after the dot. Like this

gnu/packages/messaging.scm:1253:17: address@hidden: sentences in description should be followed by two spaces; possible infraction at 68
 
+manual SSL certificate verification.")
+    (home-page "https://github.com/boothj5/libmesode")
+    (license (list license:gpl3+ license:x11))))


Further, guix size reports this

/gnu/store/50zvxq72sxcnx8vy7h4wijbfyixyhr0f-libmesode-0.9.1         67.8     0.3   0.4%
/gnu/store/liib5wid6rx9rkss78spc7wcqzwb1g2k-openssl-1.0.2j          66.9     5.9   8.8%
/gnu/store/ppk2n4xi6pzgchcfzl2p48yj7y4jvnjv-expat-2.2.0             61.6     0.6   0.9%
/gnu/store/cdi08kw7r6r684w8mk0xq0dkgpjhfpmd-gcc-4.9.4-lib           61.0    22.7  33.5%
/gnu/store/iwgi9001dmmihrjg4rqhd6pa6788prjw-glibc-2.24              38.3    36.8  54.3%
/gnu/store/rvgmixpmsq5lqr9qflhkm70kg7a4rys2-bash-static-4.4.0        1.4     1.4   2.1%

I'm ok with all these lines but the last one: bash static. I don't know what libmesode does, but does a library need bash ?

I'd like other reviewers to chime in, here

Also I couldn't check whether the build is deteministic because I have already built it and calling "build --rounds=2" again won't reiterate the build process.
Please, help

There might be more observations, keep in mind that I'm not an expert reviewer.
 
In the meantime I suggest you to take a look at the manual page about submitting patches. That's where I took the bulleted list of checks and suggestions that I made here.
So the next time you can do it yourself !
It's here
https://www.gnu.org/software/guix/manual/html_node/Submitting-Patches.html#Submitting-Patches

Thanks for working on this !
Ciao

reply via email to

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