guix-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] gnu: Add zsh


From: Ludovic Courtès
Subject: Re: [PATCH] gnu: Add zsh
Date: Mon, 08 Sep 2014 14:01:01 +0200
User-agent: Gnus/5.130011 (Ma Gnus v0.11) Emacs/24.3 (gnu/linux)

Hello,

address@hidden skribis:

> This is my first package, so I'd be interested to know if there is things 
> that could be done better.
> For the license, most of the files uses their own license but a few are under 
> GPL, so I put that. Is that right, or should I
> put (license (x11-style "file://LICENCE")) as suggested on irc anyway ?

It seems to me that the text in LICENSE is a slight variant on the
MIT/X11 license.  Thus, ‘x11-style’ would be appropriate.

However, LICENSE also reads this:

  However, note that certain shell functions are licensed under versions
  of the GNU General Public Licence.  Anyone distributing the shell as a
  binary including those files needs to take account of this.  Search
  shell functions for "Copyright" for specific copyright information.
  None of the core functions are affected by this, so those files may
  simply be omitted.

That effectively makes the combined work GPL, so this is what we should
put in the ‘license’ field (with a comment explaining this.)

In practice, the only file under GPL is
‘Completion/Unix/Command/_darcs’.  It’s ‘gplv2+’, to be precise.


The patch looks good to me, so I’ll just point out minor stylistic
things to change before we commit (the ‘HACKING’ file has more details
on all this.)

> From 5e9276e131e744cdf5c93ad1713236e70f80797e Mon Sep 17 00:00:00 2001
> From: Kevin Lemonnier <address@hidden>
> Date: Sun, 7 Sep 2014 19:56:06 +0200
> Subject: [PATCH 2/2]   gnu: Add zsh
                       ^
Extra space here ------’

>   * gnu/packages/zsh.scm: New variables.
  ^
Same here.  Should be “New file”.

Also make sure to add the file to gnu-system.am (see 52bd0810 as an
example.)

> +;;; Copyright © 2012, 2013, 2014 Kevin Lemonnier <address@hidden>

Just “2014”.

> +    (arguments `(#:configure-flags '("--with-tcsetpgrp", "--enable-pcre")

Extraneous comma here.

> +    (synopsis "Zsh is a shell designed for interactive use, although it is 
> also a powerful scripting language")

It should be smaller, and not a sentence.  For instance:

  Powerful shell for interactive use and scripting

> +    (description "The Z shell (zsh) is a Unix shell that can be used as an 
> interactive login shell and as a powerful command interpreter for shell 
> scripting. Zsh can be thought of as an extended Bourne shell with a large 
> number of improvements, including some features of bash, ksh, and tcsh.")

Please wrap to 72 columns.

> +    (license gpl3)))

‘gpl2+’, with the comment as discussed above.

Could you post an updated patch?

Thanks in advance, and welcome on board!  :-)

Ludo’.



reply via email to

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