From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: [PATCH] Add 12 rubygems. Date: Thu, 7 Jan 2016 15:29:41 +0100 Message-ID: References: <56821E47.9010400@uq.edu.au> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46923) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHBZb-0007nj-An for guix-devel@gnu.org; Thu, 07 Jan 2016 09:29:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHBZX-0007iu-4j for guix-devel@gnu.org; Thu, 07 Jan 2016 09:29:55 -0500 Received: from sinope.bbbm.mdc-berlin.de ([141.80.25.23]:47798) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHBZW-0007gn-Kl for guix-devel@gnu.org; Thu, 07 Jan 2016 09:29:51 -0500 In-Reply-To: <56821E47.9010400@uq.edu.au>, <8760zgb8mw.fsf@elephly.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Ben Woodcroft Cc: "guix-devel@gnu.org" Here=E2=80=99s the review for the other 11 patches, starting from the las= t: > From 4ccc1fadcb67a0d296bedd51a7f73d911da4680d Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 15:13:13 +1000 > Subject: [PATCH 12/12] gnu: Add ruby-ttfunk. > * gnu/packages/ruby.scm (ruby-ttfunk): New variable. [...] > + (synopsis "Font Metrics Parser for the Prawn PDF generator") s/Metrics Parser/metrics parser/ > + (description > + "TTFunk is a TrueType font parser written in pure ruby. It is us= ed as > +part of the Prawn PDF generator.") s/ruby/Ruby/ > + ;; From the README: "Matz's terms for Ruby, GPLv2, or GPLv3. See L= ICENSE > + ;; for details." > + (license (list license:gpl2 license:gpl3 > + (license:non-copyleft > + "file://src/LICENSE" > + "See LICENSE in the distribution."))))) The license is actually just one of =E2=80=9C(list license:gpl2 license:g= pl3 license:ruby)=E2=80=9D. LICENSE just contains what appears to be the Rub= y license. > From 8a7a728c843f05206599d777fdd8c8a68f26eea4 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 15:05:18 +1000 > Subject: [PATCH 11/12] gnu: Add ruby-ascii85. > * gnu/packages/ruby.scm (ruby-ascii85): New variable. [...] > + (synopsis "Encode and decode Adobe's ascii85 binary-to-text encodi= ng") How about =E2=80=9CEncode and decode Ascii85 binary-to-text encoding=E2=80=9D > + (description > + "This library provides methods to encode and decode Adobe's Ascii= 85 > +binary-to-text encoding. The main modern use of Ascii85 is in Adobe's > +PostScript and Portable Document Format file formats.") Also here I=E2=80=99d remove =E2=80=9CAdobe=E2=80=99s=E2=80=9D. Addition= ally, you could wrap =E2=80=9CPortable Document Format=E2=80=9D in @dfn{}: "This library provides methods to encode and decode Ascii85 binary-to-text encoding. The main modern use of Ascii85 is in PostScript and @dfn{Portable Document Format} (PDF) file formats." > + (home-page > + "https://github.com/datawraith/ascii85gem") Please pull this on one line. > From 6c0f6264d58f4d1815df209ab0231c4abc054f7a Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 15:00:15 +1000 > Subject: [PATCH 10/12] gnu: Add ruby-afm. > * gnu/packages/ruby.scm (ruby-afm): New variable. [...] > + (synopsis "Read Adobe Font Metrics (afm) files") > + (description > + "@code{afm} is a library to read Adobe Font Metrics (afm) files a= nd use > +the data therein.") How about "This library provides methods to read @dfn{Adobe Font Metrics} (afm) files and use the data therein." > From 8641b00e5706f6d1fe486df457b75f91a68c5edc Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:52:04 +1000 > Subject: [PATCH 09/12] gnu: Add ruby-rc4. > * gnu/packages/ruby.scm (ruby-rc4): New variable. [...] > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (replace 'check > + (lambda _ > + (and > + (zero? (system* "rspec" "spec/rc4_spec.rb")))))))) There=E2=80=99s no need for =E2=80=98(and ...)=E2=80=99 here. > + (native-inputs > + `(("ruby-rspec" ,ruby-rspec-2))) > + (synopsis "Implementation of the RC4 algorithm") > + (description > + "RubyRC4 is a pure Ruby implementation of the RC4 algorithm.") > + ;; link on rubygems.org is dead, so use github URL. I think we can do without this comment. > From 1f13b94614029c3ca7cdc34dbbcac2d2c69e786c Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:51:34 +1000 > Subject: [PATCH 08/12] gnu: Add ruby-hashery. > * gnu/packages/ruby.scm (ruby-hashery): New variable. [...] > +(define-public ruby-hashery > + (package > + (name "ruby-hashery") > + (version "2.1.1") > + (source > + (origin > + (method url-fetch) > + (uri (rubygems-uri "hashery" version)) > + (sha256 > + (base32 > + "0xawbljsjarl9l7700bka672ixwznzwih4s9i38p1y9mp8hyx54g")))) > + (build-system ruby-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (replace 'check > + (lambda _ > + (and > + (zero? (system* "qed")) > + (zero? (system* "rubytest" "-Ilib" "-Itest" "test/")))))= ))) I would prefer moving =E2=80=98(and=E2=80=99 and =E2=80=98(zero?=E2=80=99= on the same line and then split the =E2=80=9Crubytest=E2=80=9D line. > + (native-inputs > + `(("ruby-rubytest-cli" ,ruby-rubytest-cli) > + ("ruby-qed" ,ruby-qed) > + ("ruby-lemon" ,ruby-lemon))) > + (synopsis "Hash-like classes with extra features") > + (description > + "The Hashery is a tight collection of Hash-like classes. Include= d are > +the auto-sorting Dictionary class, the efficient LRUHash, the flexible > +OpenHash and the convenient KeyHash. Nearly every class is a subclass= of the > +CRUDHash which defines a CRUD (Create, Read, Update and Delete) model = on top > +of Ruby's standard Hash making it possible to subclass and augment to = fit any > +specific use case.") Could you please wrap all class names in @code{...}? E.g. "The Hashery is a tight collection of @code{Hash}-like classes. Included are the auto-sorting @code{Dictionary} class, the efficient @code{LRUHash}, the flexible @code{OpenHash} and the convenient @code{KeyHash}. Nearly every class is a subclass of the @code{CRUDHash} which defines a CRUD (Create, Read, Update and Delete) model on top of Ruby's standard @code{Hash} making it possible to subclass and augment to fit any specific use case." > From 9d9bbcfbfa52744b2b174330164578f5129187d4 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:45:17 +1000 > Subject: [PATCH 07/12] gnu: Add ruby-rubytest-cli. > * gnu/packages/ruby.scm (ruby-rubytest-cli): New variable. [...] > + (synopsis > + "Command-line interface for rubytest") Please let them share one line. > + (description > + "Rubytest CLI is a command-line interface for running tests for > +Rubytest-based test frameworks. It provides the @code{rubytest} execu= table.") > + (home-page > + "http://rubyworks.github.io/rubytest-cli") Same here. > From 5edfa6742d7ee5a958bae3e66e978ff63f10c41a Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:42:53 +1000 > Subject: [PATCH 06/12] gnu: Add ruby-lemon. > * gnu/packages/ruby.scm (ruby-lemon): New variable. [...] > +++ b/gnu/packages/ruby.scm > @@ -3045,3 +3045,35 @@ like Test::Unit and requirement specifications s= ystems like Cucumber.") > for reuse by other test frameworks.") > (home-page "http://rubyworks.github.io/ae") > (license license:bsd-2))) > + > +(define-public ruby-lemon > + (package > + (name "ruby-lemon") > + (version "0.9.1") > + (source > + (origin > + (method url-fetch) > + (uri (rubygems-uri "lemon" version)) > + (sha256 > + (base32 > + "0gqhpgjavgpvx23rqpfqcv3d5bs8gc7lr9yvj8kxgp7mfbdc2jcm")))) The indentation here looks wrong. The paren of =E2=80=9C(base32=E2=80=9D= should be aligned with the =E2=80=9Cs=E2=80=9D of =E2=80=9Csha256=E2=80=9D, and the= opening quote of the string should be aligned with the =E2=80=9Cb=E2=80=9D of =E2=80=9Cbase32=E2=80=9D= . > + (build-system ruby-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (replace 'check > + (lambda _ > + (zero? (system* "qed"))))))) Could this all be one line starting from =E2=80=9C(replace=E2=80=9D or at= least =E2=80=9C(lambda=E2=80=9D? > + (propagated-inputs > + `(("ruby-ae" ,ruby-ae) > + ("ruby-ansi" ,ruby-ansi) > + ("ruby-rubytest" ,ruby-rubytest))) > + (native-inputs > + `(("ruby-qed" ,ruby-qed))) > + (synopsis "Test framework that correlates code structure and test > unit") You could shorten the synopsis a bit by replacing =E2=80=9Cthat correlate= s=E2=80=9D with =E2=80=9Ccorrelating=E2=80=9D. > + (description > + "Lemon is a Unit Testing Framework that enforces highly formal > +case-to-class and unit-to-method test construction. This enforcement = can help > +focus concern on individual units of behavior.") Please lowercase =E2=80=9CUnit Testing Framework=E2=80=9D. > From a6f0972e10e7fe4ede3082cc22b24c0af69f7ae9 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:37:45 +1000 > Subject: [PATCH 05/12] gnu: Add ruby-ae. > * gnu/packages/ruby.scm (ruby-ae): New variable. [...] > + (source > + (origin > + (method url-fetch) > + ;; fetch from github so tests are included ? Not sure? :) > + (uri (string-append > + "https://github.com/rubyworks/ae/archive/" > + version ".tar.gz")) > + (file-name (string-append name "-" version ".tar.gz")) > + (sha256 > + (base32 > + "147jmkx54x7asy2d8m4dyrhhf4hdx4galpnhwzai030y3cdsfrrl")))) > + (build-system ruby-build-system) > + (arguments > + `(#:phases > + (modify-phases %standard-phases > + (replace 'check > + (lambda _ > + (zero? (system* "qed"))))))) This line is short enough to merge it with the =E2=80=9C(lambda _=E2=80=9D= . > + (propagated-inputs > + `(("ruby-ansi" ,ruby-ansi))) > + (native-inputs > + `(("ruby-qed" ,ruby-qed))) > + (synopsis "Assertions library") > + (description > + "Assertive Expressive (AE) is an assertions library specifically = designed > +for reuse by other test frameworks.") That=E2=80=99s not very descriptive, but I know it=E2=80=99s difficult to= come up with good descriptions when the upstream descriptions are so vague. Oh well. Let=E2=80=99s leave it like this if you cannot come up with someth= ing a little more expressive. > From dcb24fdc106d5878c4a5ff24d2fda6e2987bb15d Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:36:20 +1000 > Subject: [PATCH 04/12] gnu: Add ruby-qed. > * gnu/packages/ruby.scm (ruby-qed): New variable. [...] > + (arguments > + ;; disable testing to break the cycle qed, ansi, qed, among other= s.Instead > + ;; simply test that the executable runs. The first line of the comment looks awfully long. There is no space after the first sentence. > + `(#:phases > + (modify-phases %standard-phases > + (replace 'check > + (lambda _ > + (zero? (system* "ruby" "-Ilib" "bin/qed" > "--copyright"))))))) Why =E2=80=9C--copyright=E2=80=9D? Are you trying to run qed to see if t= here are any errors, because there is no test suite? If so, maybe you could state your intention in a comment. > + (propagated-inputs > + `(("ruby-ansi" ,ruby-ansi) > + ("ruby-brass" ,ruby-brass))) > + (synopsis "Test framework utilizing literate programming technique= s") > + (description > + "QED (Quality Ensured Demonstrations) is a TDD (Test Driven > +Development)/BDD (Behaviour Drive Development) framework utilizing Lit= erate > +Programming techniques. QED sits somewhere between lower-level testin= g tools > +like Test::Unit and requirement specifications systems like > Cucumber.") Maybe this is better: "@dfn{Quality Ensured Demonstrations} (QED) is a test framework for @dfn{Test Driven Development} (TDD) and @dfn{Behaviour Driven Development} (BDD) utilizing Literate Programming techniques. QED sits somewhere between lower-level testing tools like @code{Test::Unit} and requirement specifications systems like Cucumber." > From 356e1fbe6f87e0da6410760a2f177021e5b0302c Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:16:42 +1000 > Subject: [PATCH 03/12] gnu: Add ruby-brass. > * gnu/packages/ruby.scm (ruby-brass): New variable. [...] > +(define-public ruby-brass > + ;; download from git so test runner is included But... we are still downloading from Rubygems. > + (package > + (name "ruby-brass") > + (version "1.2.1") > + (source > + (origin > + (method url-fetch) > + (uri (rubygems-uri "brass" version)) > + (sha256 > + (base32 > + "154lp8rp1vmg60ri1j4cb8hqlw37z7bn575h899v8hzxwi11sxka")))) > + (build-system ruby-build-system) > + (arguments > + ;; disable tests to break the cycle brass, lemon, ae, qed, brass.= Instead > + ;; simply test that the library can be require'd. Again, the comment looks a bit long; also add another space after the first sentence and capitalise =E2=80=9CDisable=E2=80=9D at the beginning. > From 81e6d9cb5470f85131e302759c3698d9c6295dd8 Mon Sep 17 00:00:00 2001 > From: Ben Woodcroft > Date: Tue, 29 Dec 2015 14:13:42 +1000 > Subject: [PATCH 02/12] gnu: Add ruby-rubytest. > * gnu/packages/ruby.scm (ruby-rubytest): New variable. [...] > + (arguments > + ;; disable testing to break the cycle rubytest, qed, brass, rubyt= est, as > + ;; well as the cycle rubytest, qed, ansi, rubytest. Instead simpl= y test > + ;; that the library can be require'd. Same as above: capitalisation and space after first sentence. > + (home-page > + "http://rubyworks.github.io/rubytest") On one line please. And that=E2=80=99s it as I reviewed the first patch already. Thanks a lo= t for the patches! Could you please send updated patches? ~~ Ricardo