Subject: Re: bug#26171: a meagre handful of pearls

From: Marius Bakke <[email-masked]>
Date: Mon, 20 Mar 2017 01:23:50 +0100

Catonano <[email-masked]> writes:

> I have considered to import ParseExcel and the patches attached here are
> dependencies.

Cool!

> The last package, called OLE::Storage_Lite, has no license
>
> The license field on the web site states "Unknown"
> See http://search.cpan.org/~jmcnamara/OLE-Storage_Lite/
>
> In the source code I couldn't find any license file

The file lib/OLE/Storage_Lite.pm mentions the use of Perl artistic
license/GPL1 in the documentation section. So I think using
(package-license perl) is fine.

The patches are almost good to go, but I have some minor nitpicks :)

Packages in perl.scm are alphabetically sorted, can you adjust the
source code location of these packages accordingly?

Also, 'perl-digest-md5' is already in there :)

> Subject: [PATCH 1/3] gnu: Add perl-crypt-rc4
                                             ^^^
Commit messages should end with a period. Additionally, they should
mention which files and variables have changed. See the commit log for
examples.

> +(define-public perl-crypt-rc4
> +  (package
> +    (name "perl-crypt-rc4")
> +    (version "2.02")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "mirror://cpan/authors/id/S/SI/SIFUKURT/Crypt-RC4-"
> +             version
> +             ".tar.gz"))
> +       (sha256
> +        (base32
> +         "1sp099cws0q225h6j4y68hmfd1lnv5877gihjs40f8n2ddf45i2y"))))
> +    (build-system perl-build-system)
> +    (home-page
> +     "http://search.cpan.org/dist/Crypt-RC4")

No need for line break here.

> +    (synopsis
> +     "Perl implementation of the RC4 encryption algorithm")

...or here.

> +    (description "A pure Perl implementation of the alleged RC4 algorithm")

I think alleged should be removed. Also descriptions should end on a
period.

> Subject: [PATCH 2/3] gnu: Add perl-digest-md5

This one can be dropped :)

> From c47dae2aa6c6bbb226e5e60dc49d448ad2a03172 Mon Sep 17 00:00:00 2001
> From: humanitiesNerd <[email-masked]>
> Date: Sun, 19 Mar 2017 09:54:30 +0100
> Subject: [PATCH 3/3] gnu: Add perl-ole-storage_lite
>
> ---
>  gnu/packages/perl.scm | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
> index 0a3769c90..73266609a 100644
> --- a/gnu/packages/perl.scm
> +++ b/gnu/packages/perl.scm
> @@ -8176,3 +8176,24 @@ in Perl source files.")
>  MD5 Message Digest algorithm from within Perl programs.")
>      (license (package-license perl))))
>  
> +(define-public perl-ole-storage_lite
> +  (package
> +    (name "perl-ole-storage_lite")

We don't normally use underscores in package names or variables. I think
a '-' works here, but not a strong opinion.

> +    (version "0.19")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "mirror://cpan/authors/id/J/JM/JMCNAMARA/OLE-Storage_Lite-"
> +             version
> +             ".tar.gz"))
> +       (sha256
> +        (base32
> +         "179cxwqxb0f9dpx8954nvwjmggxxi5ndnang41yav1dx6mf0abp7"))))
> +    (build-system perl-build-system)
> +    (home-page
> +     "http://search.cpan.org/dist/OLE-Storage_Lite")
> +    (synopsis "Read and write OLE storage files")
> +    (description "This module allows you to read and write
> +an OLE-Structured file")

It would be nice with some more information about what OLE files are.
Also check for missing periods here and in the commit message.

> +    (license (package-license perl)))) ; license unkown ?

The comment can be removed since lib/OLE/Storage_Lite.pm mentions this
license. Can you send an updated series?

TIA! :)