GNU bug report logs - #78546
[PATCH 0/3] Add iwd-service-type.

Previous Next

Full log

Message #32 received at 78546 <at> debbugs.gnu.org (full text, mbox):

From: Sergey Trofimov <sarg <at> sarg.org.ru>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Gabriel Wicki <gabriel <at> erlikon.ch>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 78546 <at> debbugs.gnu.org
Subject: Re: [bug#78546] [PATCH 3/3] services: networking: Add
 iwd-service-type.
Date: Fri, 23 May 2025 14:38:27 +0200

Hi Ludovic,

thanks for the review, I've answered a couple points below

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Sergey,
>
> Sergey Trofimov <sarg <at> sarg.org.ru> writes:
>
>> * gnu/services/networking.scm (iwd-service-type): New service type.
>> (iwd-configuration), (iwd-settings), (iwd-scan-settings),
>> (iwd-general-settings), (iwd-network-settings): New configuration types.
>> * doc/guix.texi (Networking setup): Document it.
>>
>> Change-Id: I852115b9c6768b3ec4eedb34a7f9e66438bd1429
>
> […]
>
>> +@defvar iwd-service-type
>> +This is the service type to run @acronyml{IWD,Internet Wireless Daemon},
>> +a wireless daemon required to authenticate against encrypted WiFi
>> +networks.
>
> Am I right that this conflicts with wpa-supplicant, which is what
> NetworkManager uses?  Should this be mentioned?
>

Sure, I'll also add 'wireless-daemon' to both iwd and wpa-supplicant
shepherd's provisions. This way a conflict in the config would result in
an error message.

>
>> +@deftp {Data Type} iwd-scan-settings
>> +Available @code{iwd-scan-settings} fields are:
>> +
>> +@table @asis
>> +@item @code{disable-periodic-scan} (type: maybe-boolean)
>
> As a rule of thumb, I recommend avoiding negatives for Booleans; also
> add a question mark.
>

These are the names of iwd settings, see man 5 iwd.config. I will add
question marks for booleans, but I'd rather leave the names as they're
defined by iwd authors.

>
> So this would become ‘enable-periodic-scan?’, defaulting to #t.
>
> And it can be ‘boolean’ rather than ‘maybe-boolean’ (the latter would
> default to “unset”, whose meaning isn’t clear here).
>
> Same for ‘disable-roaming-scan’.
>
>> +@item @code{initial-periodic-scan-interval} (type: maybe-number)
>> +The initial periodic scan interval upon disconnect (in seconds).
>> +
>> +@item @code{maximum-periodic-scan-interval} (type: maybe-number)
>> +The maximum periodic scan interval (in seconds).
>
> I would tend to provide a known default value and make it ‘number’
> instead of ‘maybe-number’.
>

I agree, I'll change that.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.