[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: flexbar: Enable tests.
From: |
Ricardo Wurmus |
Subject: |
Re: [PATCH] gnu: flexbar: Enable tests. |
Date: |
Mon, 27 Apr 2015 14:37:34 +0200 |
David Thompson writes:
> Ricardo Wurmus <address@hidden> writes:
>
>> this patch enables the tests of flexbar. There is no check target but a
>> validation script and some test data, so I'm just running the script
>> instead of "make check".
>
> Looks fine to me! Minor critique below:
Thanks.
>> #:phases
>> - (alist-delete 'install %standard-phases)))
>> + (alist-replace
>> + 'check
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (setenv "PATH" (string-append
>> + (assoc-ref outputs "out") "/bin:"
>> + (getenv "PATH")))
>> + (chdir "../flexbar_v2.5_src/test")
>> + (zero? (system* "bash" "flexbar_validate.sh")))
>> + (alist-delete 'install %standard-phases))))
>
> Consider rewriting using 'modify-phases' syntax.
I think in this particular case, using "modify-phases" wouldn't be much
clearer. Here we're just deleting one and replacing another phase and I
think it's as clear as things can be. Or is the use of "alist-*" in
package definitions deprecated?
(I should rewrite my icedtea7 patch, however, to use "modify-phases"
syntax. With so many custom phases it really makes sense.)
~~ Ricardo