guix-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] Add php


From: Tobias Geerinckx-Rice
Subject: Re: [PATCH] Add php
Date: Sun, 30 Oct 2016 15:03:39 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

Julien,

On 30/10/16 13:08, Julien Lepiller wrote:
> here is a patch to add php to guix.

Excellent! I see you've broken into my machine (probably through PHP),
stolen my bitrotting PHP 7 package and greatly improved it. Thanks!

An incomplete review:

> +                (chdir "ext")
[...]
> +                (chdir ".."))))

Try with-directory-excursion.

> +                "--enable-fpm" "-with-openssl"

s/-with-openssl/--with-openssl/, although the option would seem
unnecessary if the result is the same.

> +                ;"--with-snmp"

Best add a comment explaining why this is unavailable, desirable, and,
if possible, what's needed to fix it.

+        #:tests? #f))

There are tests, but many fail. This should be explained in a comment
(or fixed ;-). I keep tests enabled on my machine because I hate PHP and
like to hear it scream.

Bonus fun fact: catastrophic test failure is non-fatal and the thing
installs fine.

> +    (synopsis "PHP programming language")
> +    (description
> +      "PHP is one of the most commonly used programming language
> the web")

s/language/languages/ and a missing full stop, but it would be nice to
add even more. For example:

  PHP (PHP Hypertext Processor) is a server-side (CGI) scripting
  language designed primarily for web development but is also used as
  a general-purpose programming language.  PHP code may be embedded into
  HTML code, or it can be used in combination with various web template
  systems, web content management systems and web frameworks.

Thanks again for working on this!

Kind regards,

T G-R

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

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