guix-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 1/1] gnu: Add Anonymous Pro fonts.


From: Leo Famulari
Subject: Re: [PATCH 1/1] gnu: Add Anonymous Pro fonts.
Date: Sat, 05 Sep 2015 15:06:40 -0400


On Sat, Sep 5, 2015, at 09:34, Alex Kost wrote:
> Leo Famulari (2015-09-04 00:53 +0300) wrote:
> > +    (source (origin
> > +             (method url-fetch)
> > +             (uri (string-append 
> > "http://www.marksimonson.com/assets/content/fonts/";
> 
> This line is too long, we prefer to stay within 78 chars.  I would make
> it:
> 
>                      (string-append
>                       "http://www.marksimonson.com/assets/content/fonts/";
> 
> > +                                 "AnonymousPro-" version ".zip"))
> > +             (sha256
> > +              (base32 
> > "1asj6lykvxh46czbal7ymy2k861zlcdqpz8x3s5bbpqwlm3mhrl6"))))
> 
> Such (base32 "…") lines also look too long for me, so I prefer to move a
> hash on another line (but some people leave it this way).

I thought that lint would catch those, sorry.
 
> > +           (for-each (lambda (ttf)
> > +                       (copy-file ttf
> > +                                  (string-append font-dir "/" ttf)))
> > +                     (find-files "." "\\.ttf$"))
> > +           (for-each (lambda (doc)
> > +                       (copy-file doc
> > +                                  (string-append doc-dir "/" doc)))
> > +                       (find-files "." "\\.txt$"))))))
> 
> This (find-files …) should be shifted 2 chars left (the same as
> 'find-files' in the previous 'for-each').

No excuse there, sorry again.
 
> Otherwise, LGTM.  So if there will be no other comments and objections
> on the package name, I will push it.  No need to resend the patch — I'll
> fix these small issues I mentioned.  Thank you.

I think the package name is fine. Thank you!



reply via email to

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