Re: [PATCH] gnu: Add ruby-nokogiri

[Ricardo Wurmus <rekado@elephly.net>]

Hi Pjotr,

following are a couple of comments about the patch.  (I did not try it
myself.)

> From e2fd935a659cacde5cb74bef19406f056a262f79 Mon Sep 17 00:00:00 2001
> From: pjotrp <pjotr.public01@thebird.nl>
> Date: Mon, 13 Jul 2015 14:56:40 +0200
> Subject: [PATCH] gnu: Add ruby-nokogiri

> * gnu/packages/ruby.scm (gnu-nokogiri): New variable

The name of the variable is ‘ruby-nokogiri’, not ‘gnu-nokogiri’.  There
also should be a period at the end.

> +(define-public ruby-nokogiri
> +  (package
> +    (name "ruby-nokogiri")
> +    (version "1.6.6.2")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +		    "https://github.com/sparklemotion/nokogiri/archive/v"
> +                    version ".tar.gz"))
> +              (file-name (string-append name "-" version ".tar.gz"))
> +	      (patches (map search-patch
> +                            (list "ruby-nokogiri-Rakefile.patch")))

‘(patches ...)’ should be aligned with the previous line.  ‘(list’
should be aligned with ‘search-patch’.  Since you only have one patch,
though, I think using ‘(map search-patch’ is odd.

> +    (arguments
> +     '(
> +       #:tests? #f  ;; test fails because nokogiri can only test with a built extension (now part of install phase)

‘'(’ should not be on a line of its own.  The margin comment should only
have one semicolon and the text should be broken up to avoid long lines.

> +       #:gem-flags (list "--use-system-libraries" (string-append "--with-xml2-include=" (assoc-ref %build-inputs "libxml2") "/include/libxml2" ))

This is a very long line.  It can be broken up easily.

> +       #:phases (alist-replace
> +                 'build
> +                 (lambda _
> +		   ;; calling rake gem 2x begets a gem
> +		   (system* "rake" "gem")
> +		   (zero? (system* "rake" "gem")))
> +		  %standard-phases)))

The alignment here is off.  Also consider using the ‘modify-phases’
syntax instead.  Why exactly is “rake gem” called twice?  What is
different the second time around?

> +    (native-inputs
> +     `(("ruby-hoe" ,ruby-hoe)
> +       ("ruby-rake-compiler", ruby-rake-compiler)))
> +    (inputs
> +     `(("zlib" ,zlib)
> +       ("libxml2" ,libxml2)
> +       ("libxslt" ,libxslt)))
> +    (synopsis "Nokogiri (鋸) is an HTML, XML, SAX, and Reader parser")
> +    (description "Nokogiri parses and searches XML/HTML very quickly, and also has correctly implemented CSS3 selector support as well as XPath 1.0 support.")

Please break apart the long description into multiple lines.  In Emacs
you can do this easily with ‘fill-paragraph’ (bound to ‘M-q’ by
default).

> +    (home-page "http://www.nokogiri.org/")
> +    (license license:x11)))
> +

~~ Ricardo