Re: [PATCH] gnu: Add inklingreader

[Roel Janssen]

Hello Ricardo,

Thanks for your response.

> I wonder if the inklingreader could be added to an existing module.
> Maybe ‘graphics.scm’?

Well, InklingReader is a program with a GUI. Looking at 'graphics.scm'
seems to contain only libraries. The same seems to be true for
'image.scm'.

I'm fine with putting it in 'graphics.scm', but I don't think it belongs
there. (Correct me if I'm wrong, I'm new to Guix ;)

>> From 7cd292fdb46637858e4eaedd3570b607b5087720 Mon Sep 17 00:00:00 2001
>> From: Roel Janssen <address@hidden>
>> Date: Tue, 10 Nov 2015 15:01:33 +0100
>> Subject: [PATCH] gnu: Add inklingreader
>
> Nitpick: end the summary line with a period ‘.’.

Sorry about that. Fixed in the second version.

>> * gnu-system.am: Add the package to GNU_SYSTEM_MODULES.
>> * gnu/packages/inklingreader.scm: The package description.
>
> Normally, we write it like this:
>
>> * gnu/packages/inklingreader.scm: New file.
>> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
>
> If adding to an existing module it would be something like this:
>
>> * gnu/packages/graphics.scm (inklingreader): New variable.
>
> [...]

Sorry about that. This should be better in the second version.

>> +(define-public inklingreader
>> +  (package
>> +    (name "inklingreader")
>> +    (version "0.8")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (string-append
>> +                    "ftp://alpha.gnu.org/gnu/inklingreader/inklingreader-";
>> +                    version ".tar.gz"))
>
> You should be able to use “mirror://gnu/” here.  I don’t know the exact
> URL, though (and I haven’t tried yet).

I think this doesn't work for 'alpha.gnu.org' (at least 'guix lint'
crashes when using 'mirror://'). Other packages (like artanis) use
'ftp://alpha.gnu.org/...' as well.

If I am mistaking, please let me know what the URI should be.

>> +              (sha256
>> +               (base32
>> +                "0ikg95mgwfqh0bq4dzvkfmdiycacqvv27g91hl6adwk5y3gzl96g"))))
>> +    (build-system gnu-build-system)
>> +    (arguments `(#:configure-flags '("--enable-silent-rules")))
>
> What does this do?  (I’m just curious.)

It makes the compile command output shorter. Instead of displaying the
entire command ('gcc ...') it only displays the target name being
compiled.

I realise this may not be desired, so I removed it from the second
version. 

>> +    (inputs
>> +     `(("glib" ,glib)
>> +       ("cairo" ,cairo)
>> +       ("librsvg" ,librsvg)
>> +       ("gtk+" ,gtk+)
>> +       ("libusb" ,libusb)
>> +
>> +       ;; Needed to build the package.
>> +       ("texinfo" ,texinfo)))
>
> Inputs that are only needed at build time should be added to
> ‘native-inputs’ instead.

Thanks. I removed texinfo completely because the build process doesn't
build the documentation anyway. The inputs specified now are all runtime
dependencies.

>> +    (native-inputs
>> +     `(("pkg-config" ,pkg-config)))
>> +
>
> Please do not put an empty line here.

Fixed in the second version.

>> +    (home-page "http://savannah.gnu.org/projects/inklingreader/";)
>> +    (synopsis "Wacom Inkling sketch format conversion and manipulation")
>> +    (description
>> +     "GNU InklingReader is a free software package to support the Wacom 
>> Inkling
>> +device, including data conversion to various free formats, basic editing
>> +features, and an Inkscape plugin.")
>
> Since all software in Guix is free software you should not mention this
> here again.

Fixed in the second version.

> I wonder about the Inkscape plugin: does it work out of the box with our
> inkscape package?

It's separated from the program. To enable the Inkscape extension, one
has to copy files from the repository to
'~/.config/inkscape/extensions/' so that Inkscape can find it.

The files are not included in the release. Should I remove the mention
of the Inkscape plugin from the description?

Thanks for your time,
Roel Janssen