* [PATCH 1/3] gnu: Add cfitsio
@ 2016-08-30 19:21 John Darrington
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
To: guix-devel; +Cc: John Darrington
* gnu/packages/astronomy.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
---
gnu/local.mk | 1 +
gnu/packages/astronomy.scm | 53 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 gnu/packages/astronomy.scm
diff --git a/gnu/local.mk b/gnu/local.mk
index d75ab54..26c4151 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -42,6 +42,7 @@ GNU_SYSTEM_MODULES = \
%D%/packages/apr.scm \
%D%/packages/aspell.scm \
%D%/packages/assembly.scm \
+ %D%/packages/astronomy.scm \
%D%/packages/attr.scm \
%D%/packages/audacity.scm \
%D%/packages/audio.scm \
diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
new file mode 100644
index 0000000..881e549
--- /dev/null
+++ b/gnu/packages/astronomy.scm
@@ -0,0 +1,53 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 John Darrington <jmd@gnu.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages astronomy)
+ #:use-module (guix packages)
+ #:use-module (guix licenses)
+ #:use-module (guix download)
+ #:use-module (guix build-system gnu))
+
+(define-public cfitsio
+ (package
+ (name "cfitsio")
+ (version "3390")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append
+ "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
+ ".tar.gz"))
+ (sha256
+ (base32 "02gllydm63irwbqqisa3mrskw1fphm5rlplglz3mq9whi3rxilv2"))))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:tests? #f ; no tests
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'replace-slash-bin
+ (lambda _
+ (substitute* "Makefile.in" (("/bin/") "")))))))
+ (home-page "http://heasarc.gsfc.nasa.gov/fitsio/fitsio.html")
+ (synopsis "Library for reading and writing FITS files")
+ (description "CFITSIO provides simple high-level routines for reading and
+writing FITS (Flexible Image Transport System) files that insulate the
+programmer from the internal complexities of the FITS format. CFITSIO also
+provides many advanced features for manipulating and filtering the information
+in FITS files.")
+ (license (license:non-copyleft "file://License.txt"
+ "See License.txt in the distribution."))))
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] gnu: Add wcslib
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
@ 2016-08-30 19:21 ` John Darrington
2016-08-30 20:47 ` Leo Famulari
2016-08-31 6:42 ` Alex Kost
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
To: guix-devel; +Cc: John Darrington
* gnu/packages/astronomy.scm (wcslib): New variable.
---
gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 881e549..53e86c8 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
in FITS files.")
(license (license:non-copyleft "file://License.txt"
"See License.txt in the distribution."))))
+
+(define-public wcslib
+ (package
+ (name "wcslib")
+ (version "5.15")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append
+ "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
+ (sha256
+ (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
+ (inputs
+ `(("cfitsio" ,cfitsio)))
+ (build-system gnu-build-system)
+ (arguments
+ `(#:phases (modify-phases %standard-phases
+ (add-before 'configure 'patch-/bin/sh
+ (lambda _
+ (substitute* "makedefs.in"
+ (("/bin/sh") "sh")))))))
+ (home-page "http://www.atnf.csiro.au/people/mcalabre/WCS")
+ (synopsis "An implementation of the FITS WCS standard")
+ (description "The FITS \"World Coordinate System\" (WCS) standard defines
+keywords and usage that provide for the description of astronomical coordinate
+systems in a FITS image header.")
+ (license license:lgpl3+)))
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] gnu: Add gnuastro
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
@ 2016-08-30 19:21 ` John Darrington
2016-08-30 20:45 ` Leo Famulari
2016-08-31 6:47 ` Alex Kost
2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
2016-08-31 6:45 ` Alex Kost
3 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
To: guix-devel; +Cc: John Darrington
* gnu/packages/astronomy.scm (gnuastro): New variable.
---
gnu/packages/astronomy.scm | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 53e86c8..24c6d79 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -18,8 +18,10 @@
(define-module (gnu packages astronomy)
#:use-module (guix packages)
- #:use-module (guix licenses)
+ #:use-module ((guix licenses) #:prefix license:)
#:use-module (guix download)
+ #:use-module (gnu packages image)
+ #:use-module (gnu packages maths)
#:use-module (guix build-system gnu))
(define-public cfitsio
@@ -78,3 +80,27 @@ in FITS files.")
keywords and usage that provide for the description of astronomical coordinate
systems in a FITS image header.")
(license license:lgpl3+)))
+
+(define-public gnuastro
+ (package
+ (name "gnuastro")
+ (version "0.1")
+ (source
+ (origin
+ (method url-fetch)
+ (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
+ version ".tar.gz"))
+ (sha256
+ (base32
+ "105s007kw8l3jwwhvh8k9lgbpfbf7sqh2wpxmvpv3qdr6nh9lnjg"))))
+ (inputs
+ `(("cfitsio" ,cfitsio)
+ ("gsl" ,gsl)
+ ("libjpeg" ,libjpeg-8)
+ ("wcslib" ,wcslib)))
+ (build-system gnu-build-system)
+ (home-page "http://www.gnu.org/software/gnuastro")
+ (synopsis "Astronomical data manipulation programs")
+ (description "The GNU Astronomy Utilities (Gnuastro) is a suite of
+programs for the manipulation and analysis of astronomical data.")
+ (license license:gpl3+)))
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] gnu: Add gnuastro
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
@ 2016-08-30 20:45 ` Leo Famulari
2016-08-31 6:47 ` Alex Kost
1 sibling, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:45 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
On Tue, Aug 30, 2016 at 09:21:12PM +0200, John Darrington wrote:
> * gnu/packages/astronomy.scm (gnuastro): New variable.
> + (source
> + (origin
I would shift this one column to the right, and the rest of (source)
accordingly.
> + (method url-fetch)
> + (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
> + version ".tar.gz"))
The (uri) should be aligned with (method url-fetch). Otherwise LGTM.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] gnu: Add cfitsio
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
@ 2016-08-30 20:47 ` Leo Famulari
2016-08-31 6:45 ` Alex Kost
3 siblings, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:47 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
On Tue, Aug 30, 2016 at 09:21:10PM +0200, John Darrington wrote:
> + (uri (string-append
> + "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
> + ".tar.gz"))
I would indent this based on (string-append).
> + (add-after 'unpack 'replace-slash-bin
I would call this "patch-paths" or something like that, in order to
communicate the intention.
Otherwise LGTM.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
@ 2016-08-30 20:47 ` Leo Famulari
2016-08-31 6:42 ` Alex Kost
1 sibling, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:47 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
On Tue, Aug 30, 2016 at 09:21:11PM +0200, John Darrington wrote:
> * gnu/packages/astronomy.scm (wcslib): New variable.
LGTM
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
2016-08-30 20:47 ` Leo Famulari
@ 2016-08-31 6:42 ` Alex Kost
2016-09-12 13:44 ` Alex Kost
1 sibling, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-08-31 6:42 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
John Darrington (2016-08-30 22:21 +0300) wrote:
> * gnu/packages/astronomy.scm (wcslib): New variable.
> ---
> gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
>
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> index 881e549..53e86c8 100644
> --- a/gnu/packages/astronomy.scm
> +++ b/gnu/packages/astronomy.scm
> @@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
> in FITS files.")
> (license (license:non-copyleft "file://License.txt"
> "See License.txt in the distribution."))))
> +
> +(define-public wcslib
> + (package
> + (name "wcslib")
> + (version "5.15")
> + (source
> + (origin
> + (method url-fetch)
> + (uri (string-append
> + "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
As for me, this line is too long, I would write:
(uri (string-append
"ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
name "-" version ".tar.bz2"))
> + (sha256
> + (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
> + (inputs
> + `(("cfitsio" ,cfitsio)))
> + (build-system gnu-build-system)
> + (arguments
> + `(#:phases (modify-phases %standard-phases
> + (add-before 'configure 'patch-/bin/sh
> + (lambda _
> + (substitute* "makedefs.in"
> + (("/bin/sh") "sh")))))))
This phase should end with #t.
> + (home-page "http://www.atnf.csiro.au/people/mcalabre/WCS")
> + (synopsis "An implementation of the FITS WCS standard")
I think we don't begin synopses with articles ("guix lint" should report
about it).
> + (description "The FITS \"World Coordinate System\" (WCS) standard defines
> +keywords and usage that provide for the description of astronomical coordinate
> +systems in a FITS image header.")
^^
Please remove trailing spaces
> + (license license:lgpl3+)))
--
Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] gnu: Add cfitsio
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
` (2 preceding siblings ...)
2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
@ 2016-08-31 6:45 ` Alex Kost
3 siblings, 0 replies; 13+ messages in thread
From: Alex Kost @ 2016-08-31 6:45 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
John Darrington (2016-08-30 22:21 +0300) wrote:
> * gnu/packages/astronomy.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> ---
> gnu/local.mk | 1 +
> gnu/packages/astronomy.scm | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 54 insertions(+)
> create mode 100644 gnu/packages/astronomy.scm
>
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index d75ab54..26c4151 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -42,6 +42,7 @@ GNU_SYSTEM_MODULES = \
> %D%/packages/apr.scm \
> %D%/packages/aspell.scm \
> %D%/packages/assembly.scm \
> + %D%/packages/astronomy.scm \
> %D%/packages/attr.scm \
> %D%/packages/audacity.scm \
> %D%/packages/audio.scm \
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> new file mode 100644
> index 0000000..881e549
> --- /dev/null
> +++ b/gnu/packages/astronomy.scm
> @@ -0,0 +1,53 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 John Darrington <jmd@gnu.org>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu packages astronomy)
> + #:use-module (guix packages)
> + #:use-module (guix licenses)
> + #:use-module (guix download)
> + #:use-module (guix build-system gnu))
> +
> +(define-public cfitsio
> + (package
> + (name "cfitsio")
> + (version "3390")
> + (source
> + (origin
> + (method url-fetch)
> + (uri (string-append
> + "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
> + ".tar.gz"))
^
Please indent the above lines properly and remove the trailing space, I
would write it like this:
(uri (string-append
"http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/"
name version ".tar.gz"))
> + (sha256
> + (base32 "02gllydm63irwbqqisa3mrskw1fphm5rlplglz3mq9whi3rxilv2"))))
> + (build-system gnu-build-system)
> + (arguments
> + `(#:tests? #f ; no tests
> + #:phases
> + (modify-phases %standard-phases
> + (add-after 'unpack 'replace-slash-bin
> + (lambda _
> + (substitute* "Makefile.in" (("/bin/") "")))))))
Please add #t in the end of this phase.
> + (home-page "http://heasarc.gsfc.nasa.gov/fitsio/fitsio.html")
> + (synopsis "Library for reading and writing FITS files")
> + (description "CFITSIO provides simple high-level routines for reading and
> +writing FITS (Flexible Image Transport System) files that insulate the
> +programmer from the internal complexities of the FITS format. CFITSIO also
> +provides many advanced features for manipulating and filtering the information
> +in FITS files.")
> + (license (license:non-copyleft "file://License.txt"
> + "See License.txt in the distribution."))))
^^^^^^^^^
Indentation here
--
Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] gnu: Add gnuastro
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
2016-08-30 20:45 ` Leo Famulari
@ 2016-08-31 6:47 ` Alex Kost
1 sibling, 0 replies; 13+ messages in thread
From: Alex Kost @ 2016-08-31 6:47 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
John Darrington (2016-08-30 22:21 +0300) wrote:
> * gnu/packages/astronomy.scm (gnuastro): New variable.
> ---
> gnu/packages/astronomy.scm | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
>
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> index 53e86c8..24c6d79 100644
> --- a/gnu/packages/astronomy.scm
> +++ b/gnu/packages/astronomy.scm
> @@ -18,8 +18,10 @@
>
> (define-module (gnu packages astronomy)
> #:use-module (guix packages)
> - #:use-module (guix licenses)
> + #:use-module ((guix licenses) #:prefix license:)
> #:use-module (guix download)
> + #:use-module (gnu packages image)
> + #:use-module (gnu packages maths)
> #:use-module (guix build-system gnu))
>
> (define-public cfitsio
> @@ -78,3 +80,27 @@ in FITS files.")
> keywords and usage that provide for the description of astronomical coordinate
> systems in a FITS image header.")
> (license license:lgpl3+)))
> +
> +(define-public gnuastro
> + (package
> + (name "gnuastro")
> + (version "0.1")
> + (source
> + (origin
> + (method url-fetch)
> + (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
> + version ".tar.gz"))
^^
Please shift this (uri ...) one character to the right.
> + (sha256
> + (base32
> + "105s007kw8l3jwwhvh8k9lgbpfbf7sqh2wpxmvpv3qdr6nh9lnjg"))))
> + (inputs
> + `(("cfitsio" ,cfitsio)
> + ("gsl" ,gsl)
> + ("libjpeg" ,libjpeg-8)
> + ("wcslib" ,wcslib)))
> + (build-system gnu-build-system)
> + (home-page "http://www.gnu.org/software/gnuastro")
> + (synopsis "Astronomical data manipulation programs")
> + (description "The GNU Astronomy Utilities (Gnuastro) is a suite of
> +programs for the manipulation and analysis of astronomical data.")
> + (license license:gpl3+)))
--
Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-08-31 6:42 ` Alex Kost
@ 2016-09-12 13:44 ` Alex Kost
2016-09-12 15:40 ` John Darrington
0 siblings, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-09-12 13:44 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel
Alex Kost (2016-08-31 09:42 +0300) wrote:
> John Darrington (2016-08-30 22:21 +0300) wrote:
>
>> * gnu/packages/astronomy.scm (wcslib): New variable.
>> ---
>> gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>>
>> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
>> index 881e549..53e86c8 100644
>> --- a/gnu/packages/astronomy.scm
>> +++ b/gnu/packages/astronomy.scm
>> @@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
>> in FITS files.")
>> (license (license:non-copyleft "file://License.txt"
>> "See License.txt in the distribution."))))
>> +
>> +(define-public wcslib
>> + (package
>> + (name "wcslib")
>> + (version "5.15")
>> + (source
>> + (origin
>> + (method url-fetch)
>> + (uri (string-append
>> + "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
>
> As for me, this line is too long, I would write:
>
> (uri (string-append
> "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
> name "-" version ".tar.bz2"))
>
>> + (sha256
>> + (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
>> + (inputs
>> + `(("cfitsio" ,cfitsio)))
>> + (build-system gnu-build-system)
>> + (arguments
>> + `(#:phases (modify-phases %standard-phases
>> + (add-before 'configure 'patch-/bin/sh
>> + (lambda _
>> + (substitute* "makedefs.in"
>> + (("/bin/sh") "sh")))))))
>
> This phase should end with #t.
I've noticed that you didn't fix these things (long line and #t after
substitute*). Could please do it next time :-)
The same for 'cfitsio' package.
--
Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-09-12 13:44 ` Alex Kost
@ 2016-09-12 15:40 ` John Darrington
2016-09-13 8:10 ` Alex Kost
0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-09-12 15:40 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel, John Darrington
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
I've noticed that you didn't fix these things (long line and #t after
substitute*). Could please do it next time :-)
The same for 'cfitsio' package.
Done.
If this is important why doesn't guix build and/or guix lint check for it?
--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-09-12 15:40 ` John Darrington
@ 2016-09-13 8:10 ` Alex Kost
2016-09-13 12:44 ` John Darrington
0 siblings, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-09-13 8:10 UTC (permalink / raw)
To: John Darrington; +Cc: guix-devel, John Darrington
John Darrington (2016-09-12 17:40 +0200) wrote:
> On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
>
> I've noticed that you didn't fix these things (long line and #t after
> substitute*). Could please do it next time :-)
>
> The same for 'cfitsio' package.
>
> Done.
>
> If this is important why doesn't guix build and/or guix lint check for it?
"guix lint" can't check if a phase should end with #t or not, it's up to
you check if it is needed. The thing is: if a phase fails, it should
return false value, and if it succeeds, it should return non-false
value. A returned value of 'substitute*' is unspecified, so here you
should add #t to the end of the phase. It works without it, but I would
say it happens "by chance" because #<unspecified> is considered to be
non-false, but we shouldn't rely on it, so adding #t to such phases is
the right thing.
As for the long line, it is 89 chars long, while "guix lint" reports
about exceeding 90 chars (see 'report-long-line' in (guix scripts lint)
module). BTW I think this is too loose, I would limit it to 80.
But your line could be easily shorten, as I wrote at
<http://lists.gnu.org/archive/html/guix-devel/2016-08/msg02070.html>,
so instead of the current long line:
(uri (string-append
"ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
it could be:
(uri (string-append
"ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
name "-" version ".tar.bz2"))
which fits any screen and thus is more readable I think.
I just felt a bit of a letdown that you ignored my comments and
didn't send a message why.
--
Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] gnu: Add wcslib
2016-09-13 8:10 ` Alex Kost
@ 2016-09-13 12:44 ` John Darrington
0 siblings, 0 replies; 13+ messages in thread
From: John Darrington @ 2016-09-13 12:44 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]
On Tue, Sep 13, 2016 at 11:10:57AM +0300, Alex Kost wrote:
John Darrington (2016-09-12 17:40 +0200) wrote:
> On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
>
> I've noticed that you didn't fix these things (long line and #t after
> substitute*). Could please do it next time :-)
>
> The same for 'cfitsio' package.
>
> Done.
>
> If this is important why doesn't guix build and/or guix lint check for it?
"guix lint" can't check if a phase should end with #t or not, it's up to
you check if it is needed. The thing is: if a phase fails, it should
return false value, and if it succeeds, it should return non-false
value. A returned value of 'substitute*' is unspecified, so here you
should add #t to the end of the phase. It works without it, but I would
say it happens "by chance" because #<unspecified> is considered to be
non-false, but we shouldn't rely on it, so adding #t to such phases is
the right thing.
1. Presumably guix build somewhere calls something like:
(if (run-phase x) (run-next-phase) (error))
We could change this to:
(if (let ((result (run-phase x)))
(if (unspecfied? result) (error "Result of phase is unspecified"))
result)
(run-next-phase)
(error))
.... or something similar ...
2. It wouldn't be a bad idea to change subsitute* to return something. For
example, the number of substitutions performed.
As for the long line, it is 89 chars long, while "guix lint" reports
about exceeding 90 chars (see 'report-long-line' in (guix scripts lint)
module). BTW I think this is too loose, I would limit it to 80.
But your line could be easily shorten, as I wrote at
<http://lists.gnu.org/archive/html/guix-devel/2016-08/msg02070.html>,
so instead of the current long line:
(uri (string-append
"ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
it could be:
(uri (string-append
"ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
name "-" version ".tar.bz2"))
which fits any screen and thus is more readable I think.
I think all the lines are less than 80 right now aren't they?
I just felt a bit of a letdown that you ignored my comments and
didn't send a message why.
I didn't mean to shun you. I appreciate the time and effort you took to
review my code.
Unfortunately I find the Guix workflow awkward to manage so sometimes I
have omitted corrections which ought to have been made. Part of my
problem is that currently (due to lack of many services) I cannot send or
receive mail from the machine on which I have GuixSD running. Hopefully
this will change soon.
J'
--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-09-13 12:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
2016-08-30 20:47 ` Leo Famulari
2016-08-31 6:42 ` Alex Kost
2016-09-12 13:44 ` Alex Kost
2016-09-12 15:40 ` John Darrington
2016-09-13 8:10 ` Alex Kost
2016-09-13 12:44 ` John Darrington
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
2016-08-30 20:45 ` Leo Famulari
2016-08-31 6:47 ` Alex Kost
2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
2016-08-31 6:45 ` Alex Kost
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).