From mboxrd@z Thu Jan  1 00:00:00 1970
From: Federico Beffa <beffa@ieee.org>
Subject: Re: [PATCH 2/2] import: Add stackage importer and updater.
Date: Thu, 9 Feb 2017 19:39:15 +0100
Message-ID: <CAKrPhPNS+HjmGb7_Rz8Axnc0ZB0103NMkA3ingTwhadfRgbjNg@mail.gmail.com>
References: <CAKrPhPPQKLOBd8Cma6qgS+Je7RKZZ_Emd+Kp15-LYkqd13GVkw@mail.gmail.com>
	<877f50og30.fsf@gnu.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34544)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <federico.beffa@gmail.com>) id 1cbtcm-0001Jp-CT
	for guix-devel@gnu.org; Thu, 09 Feb 2017 13:39:21 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <federico.beffa@gmail.com>) id 1cbtcl-0001JZ-HE
	for guix-devel@gnu.org; Thu, 09 Feb 2017 13:39:20 -0500
In-Reply-To: <877f50og30.fsf@gnu.org>
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= <ludo@gnu.org>
Cc: Guix-devel <guix-devel@gnu.org>

On Wed, Feb 8, 2017 at 4:30 PM, Ludovic Court=C3=A8s <ludo@gnu.org> wrote:
> Hello Federico,
>
> Federico Beffa <beffa@ieee.org> skribis:

> [...]

>> +    (if name-version
>> +        (hackage->guix-package name-version
>> +                               #:include-test-dependencies?
>> +                               include-test-dependencies?)
>> +        (begin
>> +          (format (current-error-port)
>> +                  "guix import stackage: package not found: ~a~%" packa=
ge-name)
>> +          #f))))
>
> The =E2=80=98if=E2=80=99 is unnecessary here: (guix scripts import) produ=
ces an error
> message when an importer returns something other than a =E2=80=98package=
=E2=80=99 sexp.

I've implemented all suggested changes apart from this one for two reasons:

* If the package is not included in an LTS release then the procedure
generating name-version returns #f. If I remove the 'if' then
hackage->guix-package will be called with a boolean instead of a
string as the first parameter and that will throw an exception.

* Here we can give a more accurate error message: we can say that the
package was not found in the LTS release, while, by returning, the
error would be "failed to download cabal file" which is somewhat
misleading.

Thanks for the review.
Fede