Subject: Re: bug#61587: [PATCH 0/8] networking services refactoring

From: Ludovic Courtès <[email-masked]>
Date: Tue, 07 Mar 2023 11:04:32 +0100

Hi,

Ludovic Courtès <[email-masked]> skribis:

> Bruno Victal <[email-masked]> skribis:
>
>> Note: This also makes wpa-supplicant an optional requirement.
>>
>> * gnu/services/networking.scm (<network-manager-configuration>)
>> [shepherd-requirement]: New field.
>> (network-manager-shepherd-service): Honor it.
>> (network-manager-configuration-shepherd-requirement): Export accessor.
>> * doc/guix.texi (Networking Setup): Document it.
>
> [...]
>
>> +@item @code{shepherd-requirement} (default: @code{()})
>> +This option can be used to provide a list of symbols naming Shepherd services
>> +that this service will depend on, such as @code{'wpa-supplicant} or
>> +@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
>> +networks.
>
> For clarity, I’d write the list: @code{'(wpa-supplicant)}.
>
>> +  (shepherd-requirement network-manager-configuration-shepherd-requirement
>> +                        (default '()))
>
> [...]
>
>> -             (requirement (append '(user-processes dbus-system loopback)
>> -                                  (if iwd? '(iwd) '(wpa-supplicant))))
>> +             (requirement `(user-processes dbus-system loopback
>> +                            ,@shepherd-requirement
>> +                            ,@(if iwd? '(iwd) '())))
>
> To preserve backward compatibility and to provide a reasonable default
> (with working WiFi), I think the default for ‘shepherd-requirement’
> should be '(wpa-supplicant) rather than the empty list.

The rest of the patch series (v2) LGTM.

You can send an updated version of the patch above or I can fix it up on
your behalf.

Thanks!

Ludo’.