bug#63082: [PATCH 14/17] services: mpd: Obsolete 'environment-variables' field.

[Maxim Cournoyer]

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-04-28 15:27, Maxim Cournoyer wrote:
>> Rationale: Services can be extended via the simple-service mechanism instead
>> of having to expose fields on service configurations that are not directly
>> connected to the service's configuration.
>> 
>> * gnu/services/audio.scm (mpd-environment-variables-sanitizer): New 
>> sanitizer.
>> (mpd-configuration): Use it.
>> (mpd-shepherd-service): Hard code the useful environment variables inside the
>> Shepherd service.
>> ---
>
> This field shouldn't be deprecated as one of it's primary purposes is to 
> allow for
> the pulseaudio daemon configuration to be set to another one.
> What you're doing here is effectively hardcoding the pulseaudio configuration.

Our only means to declare a pulseaudio configuration
(pulseaudio-service-type) places it at this location, so it seems
reasonable to hard code it.  What use case do you have for a custom
pulseaudio configuration that pulseaudio-service-type could not cater
to?  This prevents users defining another environment variable and
forgetting to replace these, then wondering why the default pulse
configuration doesn't work, and it felt out of place to me (an
implementation detail better encapsulated).

> I'd consider this field to be within the same category as
> 'shepherd-requirement', it's for flexibility

I like the idea of more flexibility, but I don't like that these fields
need to be duplicated for each service, somewhat encumbering the view.
Perhaps we need to devise some 'always nice to have' set that would be
configurable for any service without having to expose these fields as
part of their main configuration?

-- 
Thanks,
Maxim