[Nix-dev] packaged keepassx; awaiting review

[Eelco Dolstra]

Hi,

On 06/08/2011 11:24 AM, Joachim Schiele wrote:

> i would upload the keepassx expression into svn if anyone is willing to review
> it. for comments please give me feedback to qknight on #nixos on freenode. and
> yes i got svn access so i can update it later.

Looks good, just some minor style comments:

> {stdenv, fetchurl, bzip2, qt4, libX11, xextproto, libXtst}:

Please add spaces like this:

   { stdenv, fetchurl, bzip2, qt4, libX11, xextproto, libXtst }:

> stdenv.mkDerivation rec {
>   name = "keepassx-0.4.3";
>
>   src = fetchurl {
>     url = "http://downloads.sourceforge.net/keepassx/keepassx-0.4.3.tar.gz";

You could write

   url = "http://downloads.sourceforge.net/keepassx/${name}.tar.gz";

to ensure that name and src are consistent.

>     sha256 = "cd901a0611ce57e62cf6df7eeeb1b690b5232302bdad8626994eb54adcfa1e85";
>   };
>
>   configurePhase = ''
>     qmake PREFIX=$out
>   '';
>
>   buildInputs = [ bzip2 qt4 libX11 xextproto libXtst ];
>
>   meta = {
>     description ="Qt password manager compatible with its Win32 and Pocket PC versions."

There shouldn't be a "." at the end of meta.description strings.

>     homepage = "http://http://www.keepassx.org/";

Quotes are not necessary around URLs.

>     license = "GPL-2";

This should be "GPLv2", or even better, stdenv.lib.licenses.gpl2.

-- 
Eelco Dolstra | http://www.st.ewi.tudelft.nl/~dolstra/