bug#63082: [PATCH 04/17] services: mpd: Obsolete the 'group' field.

[Maxim Cournoyer]

Hi,

Bruno Victal <mirai@makinata.eu> writes:

[...]

>>  @item @code{shepherd-requirement} (default: @code{()}) (type: 
>> list-of-symbol)
>>  This is a list of symbols naming Shepherd services that this service
>> @@ -33824,15 +33824,12 @@ Audio Services
>>  This is a list of symbols naming Shepherd services that this service
>>  will depend on.
>>
>> -@item @code{user} (default: @code{%mympd-user}) (type: user-account)
>> +@item @code{user} (type: user-account)
>>  Owner of the @command{mympd} process.
>>
>> -The default @code{%mympd-user} is a system user with the name ``mympd'',
>> -who is a part of the group @var{group} (see below).
>> -@item @code{group} (default: @code{%mympd-group}) (type: user-group)
>> -Owner group of the @command{mympd} process.
>> +@item @code{group} (default: @code{#f}) (type: boolean)
>> +Obsolete.  Do not use.
>
> I'd skip documenting obsolete fields.

'define-configuration' doesn't allow me to skip documenting a field, I
think :-).

>>
>> -The default @code{%mympd-group} is a system group with name ``mympd''.
>>  @item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: 
>> string)
>>  Where myMPD will store its data.
>>
>> @@ -33872,7 +33869,7 @@ Audio Services
>>  Override URI to myMPD.  See
>>  @uref{https://github.com/jcorporation/myMPD/issues/950}.
>>
>> -@item @code{script-acl} (default: @code{(mympd-ip-acl (allow
>> '("127.0.0.1")))}) (type: maybe-mympd-ip-acl)
>> +@item @code{script-acl} (type: maybe-mympd-ip-acl)
>>  ACL to access the myMPD script backend.
>
> Unrelated change?

>From the generated doc, which was probably lagging behind the source.

>>  (define mpd-deprecated-fields '((music-dir . music-directory)
>> @@ -242,15 +224,9 @@ (define (mpd-user-sanitizer value)
>>           (configuration-field-error #f 'user value))))
>>
>>  (define (mpd-group-sanitizer value)
>> -  (cond ((user-group? value) value)
>> -        ((string? value)
>> -         (warning (G_ "string value for 'group' is deprecated, use \
>> -user-group instead~%"))
>> -         (user-group
>> -          (inherit %mpd-group)
>> -          (name value)))
>> -        (else
>> -         (configuration-field-error #f 'group value))))
>> +  (when value
>> +    (warning (G_ "'group' in <mpd-configuration> is obsolete; ignoring~%")))
>> +  #f)
>
> You can drop the trailing #f I think.

You are right that I could, but I prefer to leave it, as sanitizer
should return a value compatible with their field declared type.

>>
>>  ;;;
>>
>> @@ -407,9 +383,10 @@ (define-configuration mpd-configuration
>>     (sanitizer mpd-user-sanitizer))
>>
>>    (group
>> -   (user-group %mpd-group)
>> -   "The group to run mpd as."
>> -   (sanitizer mpd-group-sanitizer))
>> +   (boolean #f)
>> +   "Obsolete.  Do not use."
>> +   (sanitizer mpd-group-sanitizer)
>> +   (serializer empty-serializer))
>
> You can simply use empty-serializer after (or before) sanitizer, it is
> a recognized literal for define-configuration.

I was under the impression that form had been deprecated; per (gnu
services configuration), around line 217:

--8<---------------cut here---------------start------------->8---
(warning #f (G_ "specifying serializers after documentation is \
deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
--8<---------------cut here---------------end--------------->8---

[...]

>>    (work-directory
>>     (string "/var/lib/mympd")
>> @@ -904,12 +871,9 @@ (define (mympd-shepherd-service config)
>>         (stop #~(make-kill-destructor))))))
>>
>>  (define (mympd-accounts config)
>> -  (match-record config <mympd-configuration> (user group)
>> -    ;; TODO: Deprecation code, to be removed.
>> -    (let ((user (if (eq? (user-account-group user) %lazy-group)
>> -                    (set-user-group user group)
>> -                    user)))
>> -      (list user group))))
>> +  (match-record config <mympd-configuration>
>> +    (user)
>> +    (list user)))
>
> Nitpick, personally I'm a fan of styling this part as:
>
> (match-record config <mympd-configuration> (user)
>   (list user))
>

Can't; it'd format this way in Emacs:

--8<---------------cut here---------------start------------->8---
(match-record config <mympd-configuration> (user)
              (list user))
--8<---------------cut here---------------end--------------->8---

-- 
Thanks,
Maxim