Subject: Re: [bug#62490] [PATCH] services: nginx: Make logging level configurable.

From: Maxim Cournoyer <[email-masked]>
Date: Tue, 28 Mar 2023 11:03:45 -0400

OK,

Bruno Victal <[email-masked]> writes:

> Hi Maxim,
>
> On 2023-03-28 15:12, Maxim Cournoyer wrote:
>> Bruno Victal <[email-masked]> writes:
>>>  
>>> +(define-compile-time-procedure (assert-valid-log-level (level symbol?))
>>> +  "Ensure @var{level} is one of @code{'debug}, @code{'info}, @code{'notice},
>>> +@code{'warn}, @code{'error}, @code{'crit}, @code{'alert}, or @code{'emerg}."
>>> +  (unless (memq level '(debug info notice warn error crit alert emerg))
>>> +    (raise
>>> +     (make-compound-condition
>>> +      (formatted-message (G_ "unknown log level '~a'") level)
>>> +      (condition (&error-location
>>> +                  (location
>>> +                   (source-properties->location procedure-call-location)))))))
>>> +  level)
>> 
>> It's the first time I've seen define-compile-time-procedure in actual
>> use.  Is it really necessary?  What happens if you omit wrapping
>> assert-valid-log-level with it?
>
> It will still work, provided the declaration is adjusted accordingly.
> As for the reasons and benefits of using define-compile-time-procedure,
> it's best explained at <https://logs.guix.gnu.org/guix/2023-03-20.log#131047>.

I guess it's not actually useful here, since none of the fields thunked?

As another note, the nginx-configuration record looks simple enough that
perhaps it'd best be migrated to use the define-configuration method,
which could be made as a prior commit to this change.

As a benefit it'd validate the other field types as well.

-- 
Thanks,
Maxim