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

From: Catonano <[email-masked]>
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 <[email-masked]>:

>
> +(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
> +                "0iaj56fkd5bjvqpvq3324ni895rmbj
> 1akbfqipjydnghfwaym4z6"))))
>

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: [email-masked]: 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: [email-masked]: 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: [email-masked]: 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