Hi Efraim, Efraim Flashner <[email protected]> writes:
> On Thu, 22 Oct 2015 11:17:11 -0400 > Mark H Weaver <[email protected]> wrote: > >> Efraim Flashner <[email protected]> writes: >> >> > efraim pushed a commit to branch master >> > in repository guix. >> > >> > commit 5d47eab0242d6f89a6837123141acdae68745328 >> > Author: Efraim Flashner <[email protected]> >> > Date: Thu Oct 22 13:12:07 2015 +0300 >> > >> > gnu: Add pbzip2. >> > >> > * gnu/packages/compression.scm (pbzip2): New variable. >> >> Thanks for this, but did you post it to guix-devel for review? >> It would be good to do so in the future. > Oops, definately forgot that this time. I'll be better about that in the > future. > >> >> Please see below for comments. >> >> > --- >> > gnu/packages/compression.scm | 33 +++++++++++++++++++++++++++++++++ >> > 1 files changed, 33 insertions(+), 0 deletions(-) >> > >> > diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm >> > index 941844b..0bb3919 100644 >> > --- a/gnu/packages/compression.scm >> > +++ b/gnu/packages/compression.scm >> > @@ -7,6 +7,7 @@ >> > ;;; Copyright © 2015 Ricardo Wurmus <[email protected]> >> > ;;; Copyright © 2015 Leo Famulari <[email protected]> >> > ;;; Copyright © 2015 Jeff Mickey <[email protected]> >> > +;;; Copyright © 2015 Efraim Flashner <[email protected]> >> > ;;; >> > ;;; This file is part of GNU Guix. >> > ;;; >> > @@ -225,6 +226,38 @@ decompression.") >> > "See LICENSE in the distribution.")) >> > (home-page "http://www.bzip.org/";)))) >> > >> > +(define-public pbzip2 >> > + (package >> > + (name "pbzip2") >> > + (version "1.1.12") >> > + (source (origin >> > + (method url-fetch) >> > + (uri (string-append "https://launchpad.net/pbzip2/1.1/"; >> > version >> > + "/+download/" name "-" version >> > ".tar.gz")) >> >> The quote (") on the line above should be aligned with the quote on the >> line above it. > Ok > >> >> Also, instead of hard coding "1.1" in the URI, please use >> 'version-major+minor' instead. You'll find many examples of it in >> gnu/packages/*.scm > Ok, found one :). I wasn't sure what (version-major+minor version) was before > >> >> > + (sha256 >> > + (base32 >> > + "1vk6065dv3a47p86vmp8hv3n1ygd9hraz0gq89gvzlx7lmcb6fsp")))) >> > + (build-system gnu-build-system) >> > + (inputs >> > + `(("bzip2", bzip2))) >> > + (arguments >> > + `(#:tests? #f ; no tests >> > + #:phases (modify-phases %standard-phases >> > + (replace 'configure >> > + (lambda* (#:key outputs #:allow-other-keys) >> > + (substitute* "Makefile" >> > + (("/usr") (assoc-ref outputs "out"))) >> > + #t))))) >> >> The alignment of the lines above is very confusing, to say the least. >> >> Anyway, it would be better to simply remove the 'configure' phase and >> instead add this: >> >> #:make-flags (list (string-append "PREFIX=" %output)) > That sure is shorter than what I had there before. fixed. > >> >> > + (home-page "http://compression.ca/pbzip2/";) >> > + (synopsis "Parallel bzip2 implementation") >> > + (description >> > + "Pbzip2 is a parallel implementation of the bzip2 block-sorting file >> > +compressor that uses pthreads and achieves near-linear speedup on SMP >> > machines. >> > +The output of this version is fully compatible with bzip2 v1.0.2 (ie: >> > anything >> > +compressed with pbzip2 can be decompressed with bzip2).") >> >> s/ie:/i.e./ > ok > >> >> > + (license (license:non-copyleft "file://COPYING" >> > + "See COPYING in the distribution.")))) >> >> Please align the open quotes. > ok > >> >> Thanks, >> Mark > > I'll hold onto my patch for a little bit longer to see if anyone else has any > suggestions, and then I'll push the fixes. It's been a week with no further suggestions, so I suggest that you proceed to push the fixes. Thanks, Mark
