From: Mark H Weaver <mhw@netris.org>
To: Efraim Flashner <efraim@flashner.co.il>
Cc: guix-devel@gnu.org
Subject: Re: 01/01: gnu: Add pbzip2.
Date: Fri, 30 Oct 2015 18:24:10 -0400 [thread overview]
Message-ID: <878u6kf111.fsf@netris.org> (raw)
In-Reply-To: <20151022203138.2a2d2528@debian-netbook> (Efraim Flashner's message of "Thu, 22 Oct 2015 20:31:38 +0300")
Hi Efraim,
Efraim Flashner <efraim@flashner.co.il> writes:
> On Thu, 22 Oct 2015 11:17:11 -0400
> Mark H Weaver <mhw@netris.org> wrote:
>
>> Efraim Flashner <efraim@flashner.co.il> writes:
>>
>> > efraim pushed a commit to branch master
>> > in repository guix.
>> >
>> > commit 5d47eab0242d6f89a6837123141acdae68745328
>> > Author: Efraim Flashner <efraim@flashner.co.il>
>> > 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 <rekado@elephly.net>
>> > ;;; Copyright © 2015 Leo Famulari <leo@famulari.name>
>> > ;;; Copyright © 2015 Jeff Mickey <j@codemac.net>
>> > +;;; Copyright © 2015 Efraim Flashner <efraim@flashner.co.il>
>> > ;;;
>> > ;;; 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
prev parent reply other threads:[~2015-10-30 22:24 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20151022142819.17853.79652@vcs.savannah.gnu.org>
[not found] ` <E1ZpGqw-0004gn-5w@vcs.savannah.gnu.org>
2015-10-22 15:17 ` 01/01: gnu: Add pbzip2 Mark H Weaver
2015-10-22 17:31 ` Efraim Flashner
2015-10-30 22:24 ` Mark H Weaver [this message]
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=878u6kf111.fsf@netris.org \
--to=mhw@netris.org \
--cc=efraim@flashner.co.il \
--cc=guix-devel@gnu.org \
/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.