guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] guix package: Add '--switch-generation' option.


From: Ludovic Courtès
Subject: Re: [PATCH] guix package: Add '--switch-generation' option.
Date: Mon, 06 Oct 2014 21:27:12 +0200
User-agent: Gnus/5.130011 (Ma Gnus v0.11) Emacs/24.3 (gnu/linux)

Alex Kost <address@hidden> skribis:

> A patch is attached.  Some comments:
>
> - ‘shitted-generation’ is not a very good name, I think.  Ideas?

‘shifted-generation’ is better :-), but otherwise maybe
‘relative-generation’?  No strong opinion.

> - ‘previous-generation-number’ may use ‘shifted-generation’ now:
>
> (define* (previous-generation-number profile #:optional
>                                      (number (generation-number profile)))
>   "Return the number of the generation before generation NUMBER of
> PROFILE, or 0 if none exists.  It could be NUMBER - 1, but it's not the
> case when generations have been deleted (there are \"holes\")."
>   (or (shifted-generation profile -1 number)
>       0))
>
>   Worth changing?

Yes, why not.

> - Perhaps it would be better to make 2 commits (?): one for adding
>   ‘shifted-generation’ and ‘switch-to-generation’ procedures to (guix
>   profiles) and another is for adding the “--switch-generation” option
>   itself.

Yes.

> - Also I made a couple of cosmetic changes in “guix/scripts/package.scm”:
>   * ‘filter-map’ was replaced by 'for-each' because it was called only for
>     side effects there;
>   * ‘begin’ was removed from ‘cond’.
>   I think these changes do not deserve a separate commit and may stay in
>   this patch.  Is it OK?

Several patches make it easier to reason about the changes, but it’s OK
here.  Your call.

> From 3cc52d1aade5e9723c38c0af5fa4437cbdf1a9b6 Mon Sep 17 00:00:00 2001
> From: Alex Kost <address@hidden>
> Date: Mon, 6 Oct 2014 17:35:51 +0400
> Subject: [PATCH] guix package: Add '--switch-generation' option.
>
> * doc/guix.texi (Invoking guix package): Update documentation.
> * guix/profiles.scm (shifted-generation, switch-to-generation): New
>   procedures.
> * guix/scripts/package.scm: Add '--switch-generation' option.
>   (switch-to-previous-generation): Use 'switch-to-generation'.

Could you add a test in tests/guix-package.sh?

The rest looks good to me, thanks for working on it!

Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]