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

[Alex Kost]

Ludovic Courtès (2014-10-06 23:27 +0400) wrote:

> 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.

I like ‘relative-generation’, thanks.

>> - ‘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.

Done.

>> - 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.

Done.

>> - 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.

OK, thanks, I left those changes.

>> 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!

Thanks, I've added a couple of tests.  The new patches are attached.
Further improvements (documentation may be unsatisfactory)?

0001-profiles-Add-procedures-for-switching-generations.patch
Description: Text Data

0002-guix-package-Add-switch-generation-option.patch
Description: Text Data