Subject: Re: [bug#37978] [PATCH 1/2] guix: new command "guix time-machine"

From: Ludovic Courtès <[email-masked]>
Date: Sun, 10 Nov 2019 13:00:33 +0100

Hi Konrad,

Thanks for the updated patch!

Konrad Hinsen <[email-masked]> skribis:

> * guix/scripts/time-machine.scm: New file.
> * Makefile.am: (MODULES): Add it.
> * guix/scripts/pull.scm: Export function channel-list.
> * guix/inferior.scm: New function cached-guix-filetree-for-channels.
> * doc/guix.texi: Document "guix time-machine"

[...]

> +(define* (cached-guix-filetree-for-channels channels
> +                                            #:key
> +                                            (cache-directory (%inferior-cache-directory))
> +                                            (ttl (* 3600 24 30)))
> +  "Return a directory containing a guix filetree defined by CHANNELS, a list of channels.
> +The directory is a subdirectory of CACHE-DIRECTORY, where entries can be reclaimed after TTL seconds.
> +This procedure opens a new connection to the build daemon."
>    (with-store store

It’s the same as in v1, right?

How about (1) calling it ‘cached-channel-instance’ (or similar; as a
rule of thumb, I try to avoid “guix” in identifiers as well as
neologisms), and (2) not opening a connection to the daemon?  :-)

As it stands, this procedure opens a connection unconditionally anyway,
so it’s fine IMO to just move that ‘with-store’ to time-machine.scm and
to ‘inferior-for-channels’.

I also think it would be preferable to make it a separate patch
(separate from the one that adds time-machine.scm), if it’s OK for you.

Thoughts?

Thank you!

Ludo’.