Subject: Re: [PATCH] services: messaging: Prosody config supports file-like objects.
From: Clément Lassieur <[email-masked]>
To: Ludovic Courtès <[email-masked]>
Cc: [email-masked]
Date: Sat, 03 Mar 2018 18:38:22 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Ludovic Courtès <[email-masked]> writes:

>> +(define (file-like? val)
>> +  (and (struct? val) ((@@ (guix gexp) lookup-compiler) val)))
>
> I’m not fond of the idea of using @@ in real code.  :-)
>
> Could you move ‘file-like?’ to (guix gexp), with a docstring, and with
> ‘->bool’ to avoid exposing internal details:
>
>   (define (file-like? value)
>     (and (struct? value) (->bool (lookup-compiler value))))
>
> ?

I did it.

>> +(define (file-object? val)
>> +  (or (file-like? val) (file-name? val)))
>
> Do we need this predicate?  After all, all we can say is that a file
> name is necessarily a string (or a string-valued gexp), but a string is
> not necessarily a file name (IOW there’s no disjoint type for file
> names.)
>
> I suppose the configuration mechanism needs it though, right?  In that
> context it’s probably OK.

The problem is that the check happens when the user evaluates
(prosody-configuration), which allow them to know right away about type
errors, without running "guix system reconfigure".  So the user doesn't
even need a store.

Plus, not everything should go to the store.  For example, certificate
keys shouldn't, as they are private.  Thus, having both 'file-object?'
and 'file-name?' makes sense to me.

> Apart from this the patch LGTM, thank you!

Pushed as bdcf0e6fd484a54240a98ddf8b6fa433c1b9bd6c, with the
modifications you suggested.

Thank you for the review!
Clément