From: Steve Sprang <steve.sprang@gmail.com>
To: Ricardo Wurmus <rekado@elephly.net>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: Add figlet.
Date: Mon, 17 Aug 2015 15:47:14 -0700 [thread overview]
Message-ID: <CA+xn8YBwTMciNWTZ4vg-9X09AggnjarFRYy-o2Yu_oU=XNxhAw@mail.gmail.com> (raw)
In-Reply-To: <87twrzom7t.fsf@elephly.net>
[-- Attachment #1.1: Type: text/plain, Size: 3016 bytes --]
Thanks for the feedback! Here's my 2nd attempt.
-Steve
On Sat, Aug 15, 2015 at 10:07 PM, Ricardo Wurmus <rekado@elephly.net> wrote:
> Hi Steve,
>
> thank you for your first Guix package! It looks great! I do have a
> couple of cosmetic comments, though.
>
> > From 1abd103363bb12e7b8aa5f5ad1329c94b0af48e8 Mon Sep 17 00:00:00 2001
> > From: Steve Sprang <scs@stevesprang.com>
> > Date: Sat, 15 Aug 2015 20:08:38 -0700
> > Subject: [PATCH] gnu: Add figlet.
>
> > * gnu/packages/figlet.scm: New file.
> > * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
>
> Looks good. I just wonder if figlet could not be added to some existing
> module instead of creating a new one. Maybe “fontutils.scm”?
>
> > + (version "2.2.5")
> > + (source
> > + (origin
> > + (method url-fetch)
> > + (uri (string-append
> > + "ftp://ftp.figlet.org/pub/figlet/program/unix/figlet-"
> version ".tar.gz"))
>
> This line is a bit long. How about this instead:
>
> (uri (string-append "ftp://ftp.figlet.org/pub/figlet/program"
> "/unix/figlet-" version ".tar.gz"))
>
> > + (sha256 (base32
> > +
> "0za1ax15x7myjl8jz271ybly8ln9kb9zhm1gf6rdlxzhs07w925z"))))
>
> I think it would look better like this:
>
> (sha256
> (base32 "0za1ax15x7myjl8jz271ybly8ln9kb9zhm1gf6rdlxzhs07w925z"))
>
> > + (build-system gnu-build-system)
> > + (arguments
> > + `(#:phases
> > + (alist-replace
> > + 'configure
> > + (lambda* (#:key outputs #:allow-other-keys)
> > + (let ((out (assoc-ref outputs "out")))
> > + (substitute* "Makefile"
> > + (("/usr/local") out))))
> > + %standard-phases)))
>
> I suggest using ‘(modify-phases ...)’ instead, as adding or removing
> phases later on does not alter the indentation of other phases.
>
> However, in this case ‘/usr/local’ doesn’t have to be patched away at
> all. You could just pass a make-flag to set ‘prefix’ to ‘(assoc-ref
> outputs "out")’.
>
> > + (synopsis "Program for making large letters out of ordinary text")
> > + (description "FIGlet is a program for making large letters out of
> ordinary text.")
>
> This line is too long. You can automatically format it in Emacs with
> ‘M-q’. Maybe the description could be a little longer to explain that
> what figlet generates is some sort of ASCII art letters, because this
> description could be misunderstood as operating on fonts. Or maybe it’s
> just me being dense ;)
>
> > + (home-page "http://www.figlet.org/")
> > + (license bsd-3)))
>
> Looking at the sources it looks like not all files are under the BSD-3
> license. ‘inflate.c’, for example, is released under expat/X11;
> ‘getopt.c’ is public domain software — I’m not sure if this warrants
> using a list for the license field. Maybe someone else could weigh in
> on this.
>
> ~~ Ricardo
>
>
[-- Attachment #1.2: Type: text/html, Size: 4285 bytes --]
[-- Attachment #2: add-figlet-2.patch --]
[-- Type: text/x-patch, Size: 2898 bytes --]
From 4e95678b96845418ef5d11a9b1c40f9beaaf85be Mon Sep 17 00:00:00 2001
From: Steve Sprang <scs@stevesprang.com>
Date: Sun, 16 Aug 2015 20:43:07 -0700
Subject: [PATCH] gnu: Add figlet.
* gnu/packages/figlet.scm: New file.
* gnu-system.am (GNU_SYSTEM_MODULES): Add it.
---
gnu-system.am | 1 +
gnu/packages/figlet.scm | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
create mode 100644 gnu/packages/figlet.scm
diff --git a/gnu-system.am b/gnu-system.am
index 9f46f7b..bd053b7 100644
--- a/gnu-system.am
+++ b/gnu-system.am
@@ -96,6 +96,7 @@ GNU_SYSTEM_MODULES = \
gnu/packages/enlightenment.scm \
gnu/packages/fcitx.scm \
gnu/packages/feh.scm \
+ gnu/packages/figlet.scm \
gnu/packages/file.scm \
gnu/packages/firmware.scm \
gnu/packages/fish.scm \
diff --git a/gnu/packages/figlet.scm b/gnu/packages/figlet.scm
new file mode 100644
index 0000000..f12c0c2
--- /dev/null
+++ b/gnu/packages/figlet.scm
@@ -0,0 +1,47 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2015 Steve Sprang <scs@stevesprang.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages figlet)
+ #:use-module ((guix licenses) #:prefix license:)
+ #:use-module (guix packages)
+ #:use-module (guix download)
+ #:use-module (guix build-system gnu))
+
+(define-public figlet
+ (package
+ (name "figlet")
+ (version "2.2.5")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append "ftp://ftp.figlet.org/pub/figlet/program"
+ "/unix/figlet-" version ".tar.gz"))
+ (sha256
+ (base32 "0za1ax15x7myjl8jz271ybly8ln9kb9zhm1gf6rdlxzhs07w925z"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:phases
+ (modify-phases %standard-phases (delete 'configure))
+ #:make-flags
+ (list (string-append "prefix=" %output))))
+ (home-page "http://www.figlet.org/")
+ (synopsis "Program for making large letterforms out of ordinary screen
+characters")
+ (description "FIGlet is a program for making large ASCII art letterforms
+out of ordinary screen characters.")
+ (license license:bsd-3)))
--
2.4.3
next prev parent reply other threads:[~2015-08-17 22:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-16 3:24 [PATCH] gnu: Add figlet Steve Sprang
2015-08-16 5:07 ` Ricardo Wurmus
2015-08-17 22:47 ` Steve Sprang [this message]
2015-08-18 5:48 ` Ricardo Wurmus
2015-08-19 22:43 ` Ludovic Courtès
2015-08-20 6:34 ` Ricardo Wurmus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+xn8YBwTMciNWTZ4vg-9X09AggnjarFRYy-o2Yu_oU=XNxhAw@mail.gmail.com' \
--to=steve.sprang@gmail.com \
--cc=guix-devel@gnu.org \
--cc=rekado@elephly.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.