Subject: Re: [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.

From: Bruno Victal <[email-masked]>
Date: Sat, 25 Mar 2023 00:33:30 +0000

On 2023-03-24 16:03, Maxim Cournoyer wrote:> Bruno Victal <[email-masked]> writes:
>>  
>> +;; XXX: These will shadow the previous definition used by mpd
>> +;;      and cause warnings to be shown. Maybe split the file
>> +;;      into audio/mpd.scm and audio/mympd.scm ?
>> +#;(define-maybe/no-serialization user-account)
>> +#;(define-maybe/no-serialization user-group)
> 
> I'd rather keeping them together if possible; could the prefix trick be
> used with them?  No need for a hanging indent for continued text, here
> and for the other occurrences.

The prefix trick is unlikely to help since the previous ones already use it.
--8<---------------cut here---------------start------------->8---
(define-maybe user-account (prefix mpd-))
(define-maybe user-group (prefix mpd-))
--8<---------------cut here---------------end--------------->8---

I'm using define-maybe as a “cheat” here for prettier documentation generation.
Without define-maybe, the documentation is rendered as:

--8<---------------cut here---------------start------------->8---
@item @code{user} (type: user-account)
The user to run mpd as.
--8<---------------cut here---------------end--------------->8---

… which is the documentation format for a field that requires an explicit value,
even though we are setting a default one using %mpd-account.

On the other hand, using define-maybe yields:

--8<---------------cut here---------------start------------->8---
@item @code{user} (type: maybe-user-account)
The user to run mpd as.
--8<---------------cut here---------------end--------------->8---

… which is the format for fields that do not require any manual intervention.

> The expressions commented; should they be?  On another note, '#;'
> appears undocumented, I'd avoid it until it is (and it's not necessary
> here).

They're commented because the “right way of things” would have that the first
maybe-user-account is mpd-configuration specific due to (prefix mpd-) and that
we should have another maybe-user-account that is unrelated.
Here, we're actually reusing the one from mpd and it happens to be “ok” since
we do no serialization due to define-configuration/no-serialization.

Regarding #;, it is documented, though perhaps in a indirect manner. It's SRFI-62,
which is listed as supported in Guile manual.

>> +          (inherit %mympd-group)
>> +          (name value)))
>> +        (else
>> +         (configuration-field-error #f 'group value))))
>> +;;;
> 
> Was this ';;;' added by mistake?

It serves as an aid to demarcate the deprecation helpers to be erased when the
time arrives. The helpers for MPD are also similarly demarcated.