guix-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] Add 12 rubygems.


From: Ricardo Wurmus
Subject: Re: [PATCH] Add 12 rubygems.
Date: Wed, 30 Dec 2015 10:14:47 +0100
User-agent: mu4e 0.9.13; emacs 24.5.1

Ben Woodcroft <address@hidden> writes:

> These patches are all working towards the popular Ruby web frameworks 
> (Rails/Sinatra). I tried where possible to do at least rudimentary 
> testing when dependency cycles arose, and fall back to the source code 
> on github when the gem on rubygems didn't contain tests.

Thank you!

> From 24edad0eac9972e946b80e5919adc99d4fe11d8b Mon Sep 17 00:00:00 2001
> From: Ben Woodcroft <address@hidden>
> Date: Tue, 29 Dec 2015 14:11:21 +1000
> Subject: [PATCH 01/12] gnu: Add ruby-ansi.

> * gnu/packages/ruby.scm (ruby-ansi): New variable.
> ---
>  gnu/packages/ruby.scm | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)

> +
> +(define-public ruby-ansi
> +  (package
> +    (name "ruby-ansi")
> +    (version "1.5.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       ;; fetch from github as the gem does not contain testing code

“github” or “GitHub”?  I think it should be the latter.

> +       (uri (string-append
> +             "https://github.com/rubyworks/ansi/archive/";
> +             version ".tar.gz"))

I’m still not happy with breaking lines right after “string-append”, but
I’ll just stop mentioning it :)  It’s not a problem, but it does look weird
to me.

> +       (file-name (string-append name "-" version ".tar.gz"))
> +       (sha256
> +        (base32
> +         "1zdip30hivyipi8hndhb457bhiz033awd00bgrsk5axjrwp6zhly"))))
> +    (build-system ruby-build-system)
> +    (arguments
> +     ;; disable testing to break the cycle ansi, ae, ansi, as well as the 
> cycle
> +     ;; ansi, qed, ansi. Instead simply test that the library can be 
> require'd.

Please use two spaces between sentences.  Also capitalise “disable”.
The comment should better be moved right above ‘(replace 'check ...)’,
so that it stays relevant even when new, unrelated build phases are
added.

> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (replace 'check
> +           (lambda _
> +             (zero? (system* "ruby" "-Ilib" "-r" "ansi")))))))
> +    (synopsis "ANSI escape code related libraries")
> +    (description
> +     "The ANSI project is a collection of ANSI escape code related libraries

I think “This package” would be less confusing than “The ANSI project”.
At first I thought this sentence describes where the ANSI code standard
comes from; but then I realized that the package is also called “ansi”.

> +enabling ANSI colorization and stylization of console output.  Included in 
> the
> +library are the Code module, which defines ANSI codes as constants and

“the @code{Code} module”

> +methods, a Mixin module for including color methods, a Logger, a ProgressBar,
> +and a String subclass.  The library also includes a Terminal module which
> +provides information about the current output device.")

Please wrap all module names in address@hidden, e.g. “a @code{Mixin}
module”.

I’ll try to comment on the other patches later.

~~ Ricardo




reply via email to

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