Subject: Re: bug#49610: [PATCH 0/2] Add channels field to guix-configuration

From: Ludovic Courtès <[email-masked]>
Date: Wed, 21 Jul 2021 23:53:36 +0200

Brice Waegeneire <[email-masked]> skribis:

> * doc/guix.texi (Channels): Specify that '/etc/guix/channels.scm'
>   contains channels configuration.
>   (Base Services): Document 'guix-configuration-channels' field.
> * gnu/services/base.scm (setup-channels): New procedure.
>   (guix-configuration): Add channels field.
>   (guix-activation): Use 'setup-channels' procedure.

[...]

> +@item @code{channels} (default: @code{'()})
> +List of system channels to use, it populates
> +@file{/etc/guix/channels.scm}.

What about:

  List of channels to be used by @command{guix pull}, by default.
  Channels listed here are written to @file{/etc/guix/channels.scm}.

?

> +;; FIXME Does this gexp should be build before boot, such as
> +;; substitute-key-authorization does?

There’s a grammatical issue :-), but also I’m not sure: what are you
worried about?

> +(define (setup-channels channels)
> +  "Return a gexp with code to setup CHANNELS, a list of channels"

Missing period.  For the name, how about ‘install-channels-file’
instead?

> +  (channels         guix-configuration-channels ;list of channels
> +                    (default '()))

I wonder if it should default to ‘%default-channels’, for consistency
and least-surprise.  In practice, it means we’d always end up creating
/etc/guix/channels.scm, but that’s probably OK.  (The downside is if we,
Guix devs, choose to change ‘%default-channels’ at some point: users
would be stuck with the value that got written to /etc.  That’s a very
hypothetical situation though.)

WDYT?

> +        #$(if (not (null? channels))
> +              (setup-channels channels)
>                #~#f))))

In that case, we could remove the (null? channels) special case.

Thanks,
Ludo’.