all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: Leo Famulari <leo@famulari.name>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH 1/1] gnu: Add dvtm.
Date: Fri, 21 Aug 2015 18:35:08 -0400	[thread overview]
Message-ID: <87mvxkb78j.fsf@netris.org> (raw)
In-Reply-To: <1439873749-27812-2-git-send-email-leo@famulari.name> (Leo Famulari's message of "Tue, 18 Aug 2015 00:55:49 -0400")

Hi!

Leo Famulari <leo@famulari.name> writes:
> * gnu/packages/dvtm.scm: New file.
> * gnu-system.am: Add it.

Please include the Makefile variable being changed, like this:

* gnu-system.am (GNU_SYSTEM_MODULES): Add it.

> diff --git a/gnu/packages/dvtm.scm b/gnu/packages/dvtm.scm
> new file mode 100644
> index 0000000..83ee293
> --- /dev/null
> +++ b/gnu/packages/dvtm.scm
> @@ -0,0 +1,29 @@
> +(define-module (gnu packages dvtm)

This new file needs the same header as our other source files, but with
only your own copyright notice on top.

> +  #:use-module (guix packages)
> +  #:use-module (guix download)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix licenses)
> +  #:use-module (gnu packages ncurses))
> +
> +(define-public dvtm 
> +  (package
> +   (name "dvtm")
> +   (version "0.14")
> +   (source (origin
> +	    (method url-fetch)

Please don't use any tabs in *.scm files.

> +	    (uri (string-append "http://www.brain-dump.org/projects/dvtm/dvtm-" version".tar.gz"))

Please keep all lines in *.scm files to 80 columns or less.  Also, there
should be a space after "version".

> +	    (sha256
> +	     (base32 "0ykl8dz7ivjgdzhmhlgidnp2ffh5gxq9lbg276w7iid4z10v76wa"))))
> +   (build-system gnu-build-system)
> + 	  (arguments
> +	    `(#:make-flags (list (string-append "PREFIX=" (assoc-ref %outputs "out")))

"(arguments" should be lined up with "(build-system", and the "`" should
be below the 'a'.  

> +	      #:phases (alist-replace 'configure (lambda _ (setenv "CC" (which "gcc"))) %standard-phases)

You should be able to remove this line and simply add "CC=gcc" to the
make flags.

> +	      #:tests? #f))

Please add a comment saying why tests are disabled.  It could be as
simple as:

      #:tests? #f))  ; no test suite

> +
> +   (inputs `(("ncurses" ,ncurses)))
> +   (synopsis "Tiling window management for the console")
> +   (description "dvtm brings the concept of tiling window management, 
> +popularized by X11-window managers like dwm to the console.  As a console window 
> +manager it tries to make it easy to work with multiple console based programs.  ")

There should probably be a comma after "dwm" here.  Please remove the
extraneous spaces at the end of the description, and wrap to fit in 80
columns.

> +   (home-page "http://www.brain-dump.org/projects/dvtm/")
> +   (license (list isc x11))))

When there is more than one license, we add a comment above the
'license' field explaining the license situation.  We need to do this
because the meaning of our license lists is ambiguous: it might mean
dual-licensing, or it might mean that some of the code is under one
license and some is under another.

Also, it should be the 'expat' license, not the 'x11' license.  They are
both often ambiguously called the "mit" license, and are very similar,
so it's a common mistake.

Can you send an updated patch?

     Thanks,
       Mark

  reply	other threads:[~2015-08-21 22:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-18  4:55 [PATCH 0/1] Adding new package: dvtm Leo Famulari
2015-08-18  4:55 ` [PATCH 1/1] gnu: Add dvtm Leo Famulari
2015-08-21 22:35   ` Mark H Weaver [this message]
2015-08-22  0:46     ` Leo Famulari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mvxkb78j.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=guix-devel@gnu.org \
    --cc=leo@famulari.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.