Subject: Re: bug#54352: [PATCH] services: dnsmasq: Add more options.

From: Ludovic Courtès <[email-masked]>
Date: Tue, 22 Mar 2022 11:02:29 +0100

Hi,

Remco van 't Veer <[email-masked]> skribis:

> 2022/03/21 19:36, Maxime Devos:
>
>> [[PGP Signed Part:Undecided]]
>> Ludovic Courtès schreef op ma 21-03-2022 om 16:22 [+0100]:
>>> I think this suggestion is beyond the scope of this review: we’ve never
>>> used sanitizers like this before (or almost), and this particular piece
>>> of code doesn’t use them.
>>>
>>> Also, with the recent discussion about the introduction of contracts,
>>> I’d rather wait an use contracts everywhere once they’re available.
>>
>> Seems reasonable to me, given that the specifics weren't discussed yet,
>> although _everywhere_ (for all procedures, records, ...) seems a bit
>> much, unless you meant every field of the dnsmasq record.

Sorry, I wasn’t clear.  I mean, when we have contracts in Guix, we can
start using them “everywhere” in Guix, primarily for configuration
records.  But we’re not there yet and how exactly we’ll get there
remains to be seen.

> I can add something like the following:
>
>   (define (assert-boolean value)
>     (unless (false-if-exception (boolean? value))
>       (error-out (format #f "expected a boolean, got: ~s" value)))
>     value)
>
> and use it to do
>
>   (sanitize assert-boolean)

My suggestion is to just do nothing for the dnsmasq record; just leave a
comment about the expected type next to it and in the manual, as is done
for almost every other configuration record.

Eventually we’ll turn that into contracts with good error reporting, I
hope, but that’s not something you can do right now.  :-)

Thanks!

Ludo’.