Subject: Re: [PATCH v5] services: network-manager: Add extra-configuration-files field.
From: Maxim Cournoyer <[email-masked]>
To: 45mg <[email-masked]>
Cc: [email-masked], Arnaud Daby-Seesaram <[email-masked]>, Ludovic Courtès <[email-masked]>
Date: Thu, 13 Feb 2025 01:05:09 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello!

45mg <[email-masked]> writes:

> Allow users to specify additional configuration files for
> NetworkManager. These files will be added to
> `/etc/NetworkManager/conf.d` (NetworkManager's default configuration
> directory location).
>
> * gnu/services/networking.scm (<network-manager-configuration>)
> [extra-configuration-files]: New field.
> (network-manager-activation): Honor the new field.
> * doc/guix.texi (Networking Setup): Document the new field.
>
> Change-Id: I07479958e4d0aa318328c666a9630b779230b300

LGTM.  I've taken the liberty to apply the following nitpick edits:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -21639,13 +21639,13 @@ Networking Setup
 @item @code{extra-configuration-files} (default: @code{'()})
 A list of two-element lists; the first element of each list is a file
 name (as a string), and the second is a file-like object.  Used to
-specify configuration files which will be added to
+specify configuration files which will be added to the
 @file{/etc/NetworkManager/conf.d}.  NetworkManager will read additional
 configuration from this directory.  For details on configuration file
-precedence and the configuration file format, see the
-@command{NetworkManager.conf(5)} man page.
+precedence and the configuration file format, see @samp{man 5
+NetworkManager.conf}.
 
-For example, to add two files @file{001-basic.conf} and
+For example, to add two files named @file{001-basic.conf} and
 @file{002-unmanaged.conf}:
 
 @lisp
@@ -21655,8 +21655,7 @@ Networking Setup
            `(("existing-file" ,(local-file "001-basic.conf"))
              ("constructed-file" ,(plain-file "002-unmanaged.conf"
                                               "[keyfile]
-unmanaged-devices=interface-name:wlo1_ap
-"))))))
+unmanaged-devices=interface-name:wlo1_ap\n"))))))
 @end lisp
 
 @end table
modified   gnu/services/networking.scm
@@ -1255,8 +1255,9 @@ (define-record-type* <network-manager-configuration>
   (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
         (default #f)
         (sanitize warn-iwd?-field-deprecation))
-  (extra-configuration-files network-manager-configuration-extra-configuration-files
-                             (default '())))  ;'((file-name-string file-like-object) ...)
+  (extra-configuration-files
+   network-manager-configuration-extra-configuration-files
+   (default '())))                 ;'((file-name-string file-like-object) ...)
 
 (define (network-manager-activation config)
   ;; Activation gexp for NetworkManager
--8<---------------cut here---------------end--------------->8---

I hope that's fine.

Finally pushed as commit 0caba8f5db!  Thank you for contributing to Guix!

--
Maxim