Re: [PATCH 2/3] gnu: Add wcslib

[Alex Kost]

John Darrington (2016-09-12 17:40 +0200) wrote:

> On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
>
>      I've noticed that you didn't fix these things (long line and #t after
>      substitute*).  Could please do it next time :-)
>
>      The same for 'cfitsio' package.
>
> Done.
>
> If this is important why doesn't guix build and/or guix lint check for it?

"guix lint" can't check if a phase should end with #t or not, it's up to
you check if it is needed.  The thing is: if a phase fails, it should
return false value, and if it succeeds, it should return non-false
value.  A returned value of 'substitute*' is unspecified, so here you
should add #t to the end of the phase.  It works without it, but I would
say it happens "by chance" because #<unspecified> is considered to be
non-false, but we shouldn't rely on it, so adding #t to such phases is
the right thing.

As for the long line, it is 89 chars long, while "guix lint" reports
about exceeding 90 chars (see 'report-long-line' in (guix scripts lint)
module).  BTW I think this is too loose, I would limit it to 80.

But your line could be easily shorten, as I wrote at
<http://lists.gnu.org/archive/html/guix-devel/2016-08/msg02070.html>,
so instead of the current long line:

       (uri (string-append
             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"; name "-" version 
".tar.bz2"))

it could be:

       (uri (string-append
             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/";
             name "-" version ".tar.bz2"))

which fits any screen and thus is more readable I think.

I just felt a bit of a letdown that you ignored my comments and
didn't send a message why.

-- 
Alex