unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: Nikita Karetnikov <nikita@karetnikov.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] guix package: Add '--delete-generations'.
Date: Sun, 22 Sep 2013 22:55:02 +0200	[thread overview]
Message-ID: <878uyo60gp.fsf@gnu.org> (raw)
In-Reply-To: <87li2oslzh.fsf_-_@karetnikov.org> (Nikita Karetnikov's message of "Sun, 22 Sep 2013 23:19:14 +0400")

Nikita Karetnikov <nikita@karetnikov.org> skribis:

> Can I push this patch to ‘master’?  Do you see any problems?

Looks good!  Minor issues discussed below.

> I had noticed that ‘--roll-back’ doesn’t output anything with
> ‘--dry-run’, so I implemented ‘--delete-generations’ similarly.  Maybe
> it would be better to print something.  WDYT?

Agreed (in a separate patch.)

> +@item --delete-generations[=@var{pattern}]
> +@itemx -d [@var{pattern}]
> +Delete generations.

“Delete the generations matching @var{patterns} or ... when omitted.”

Or what actually?  I would expect it to delete all the generations but
the current one when PATTERN is omitted, right?

> +(define (link-to-empty-environment generation)
> +  "Link GENERATION, a string, to the empty environment."

s/environment/profile/ maybe?  (I know there are other inconsistencies
in these files.)

Ideally this factorization would go in a patch of its own, before the
one that adds --delete-generations.  Is that doable for you?

> +  (define (apply-to-generations function profile pattern)

s/function/proc/ to follow the convention.

> +        (cond ((zero? number))  ; do not delete generation 0
> +              ((and (current-generation? profile generation)
> +                    (not (file-exists? previous-generation)))
> +               (begin (link-to-empty-environment previous-generation)
> +                      (switch-to-previous-generation profile)
> +                      (display-and-delete generation)))
> +              ((current-generation? profile generation)
> +               (begin (roll-back profile)
> +                      (display-and-delete generation)))

No need for ‘begin’ in the body of a ‘cond’ clause.

>      ;; First roll back if asked to.
> -    (if (and (assoc-ref opts 'roll-back?) (not dry-run?))
> -        (begin
> -          (roll-back profile)
> -          (process-actions (alist-delete 'roll-back? opts)))
> -        (let* ((installed (manifest-packages (profile-manifest profile)))
> -               (upgrade-regexps (filter-map (match-lambda
> -                                             (('upgrade . regexp)
> -                                              (make-regexp (or regexp "")))
> -                                             (_ #f))

[...]

Why is there this big hunk?  If it’s just reindenting, could you arrange
to remove this hunk?

Thanks!

Ludo’.

  parent reply	other threads:[~2013-09-22 21:00 UTC|newest]

Thread overview: 132+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-29 12:34 Goals for 0.4 Ludovic Courtès
2013-08-29 13:16 ` Nikita Karetnikov
2013-08-29 13:36   ` Ludovic Courtès
2013-08-30 17:55     ` Nikita Karetnikov
2013-08-30 18:31       ` Ludovic Courtès
2013-08-31 16:40         ` Nikita Karetnikov
2013-08-31 18:05           ` Ludovic Courtès
2013-08-31 20:34             ` Jose E. Marchesi
2013-08-31 21:07               ` Ludovic Courtès
2013-09-01 23:16                 ` New ‘--list-generations’ and ‘--delete-generations’ options (was: Goals for 0.4) Nikita Karetnikov
2013-09-02  9:08                   ` New ‘--list-generations’ and ‘--delete-generations’ options Ludovic Courtès
2013-09-05  1:30                     ` Nikita Karetnikov
2013-09-05 20:00                       ` Ludovic Courtès
2013-09-05 21:14                         ` Nikita Karetnikov
2013-09-07 19:34                           ` Ludovic Courtès
2013-09-08 10:59                             ` Nikita Karetnikov
2013-09-08 20:22                               ` Ludovic Courtès
2013-09-09  9:17                                 ` Nikita Karetnikov
2013-09-09 16:55                                   ` Ludovic Courtès
2013-09-11  5:16                                     ` Nikita Karetnikov
2013-09-11 21:25                                       ` Ludovic Courtès
2013-09-12  9:17                                         ` Nikita Karetnikov
2013-09-12 12:26                                           ` Ludovic Courtès
2013-09-13 14:44                                             ` Nikita Karetnikov
2013-09-13 21:29                                               ` Ludovic Courtès
2013-09-16 11:12                                                 ` Nikita Karetnikov
2013-09-16 12:16                                                   ` Ludovic Courtès
2013-09-18  0:43                                                     ` PRELIMINARY: [PATCH] guix package: Add '--list-generations' Nikita Karetnikov
2013-09-18 17:35                                                       ` Nikita Karetnikov
2013-09-18 21:32                                                       ` Ludovic Courtès
2013-09-19  0:49                                                         ` Nikita Karetnikov
2013-09-19  9:39                                                           ` Ludovic Courtès
2013-09-19 11:48                                                             ` Nikita Karetnikov
2013-09-19 12:13                                                               ` Ludovic Courtès
2013-09-21 20:39                                                                 ` Ludovic Courtès
2013-09-18 21:35                                                       ` PRELIMINARY: " Ludovic Courtès
2013-09-22 19:19     ` [PATCH] guix package: Add '--delete-generations' Nikita Karetnikov
2013-09-22 20:15       ` Generation 0 (was: [PATCH] guix package: Add '--delete-generations'.) Nikita Karetnikov
2013-09-22 21:15         ` Generation 0 Ludovic Courtès
2013-09-23 10:14           ` Nikita Karetnikov
2013-09-23 15:42             ` Ludovic Courtès
2013-09-24  0:54           ` Nikita Karetnikov
2013-09-24  5:56             ` [PATCH] guix package: Show which generation is the current one. (was: Generation 0) Nikita Karetnikov
2013-09-24 12:45               ` [PATCH] guix package: Show which generation is the current one Ludovic Courtès
2013-09-24 13:55                 ` Nikita Karetnikov
2013-09-24 14:16                   ` Ludovic Courtès
2013-09-25  2:10                     ` Nikita Karetnikov
2013-09-25 12:51                       ` Ludovic Courtès
2013-09-24 12:43             ` Generation 0 Ludovic Courtès
2013-09-24 22:29               ` Nikita Karetnikov
2013-09-25 12:50                 ` Ludovic Courtès
2013-09-25 18:07                   ` Nikita Karetnikov
2013-09-25 19:24                     ` Ludovic Courtès
2013-09-26  2:18                       ` Nikita Karetnikov
2013-09-26  9:44                         ` Ludovic Courtès
2013-09-22 20:55       ` Ludovic Courtès [this message]
2013-09-23 10:11         ` [PATCH] guix package: Add '--delete-generations' Nikita Karetnikov
2013-09-23 15:41           ` Ludovic Courtès
2013-09-24  7:21             ` Nikita Karetnikov
2013-09-24 12:50               ` Ludovic Courtès
2013-09-24 13:57                 ` Nikita Karetnikov
2013-09-25  4:21             ` Nikita Karetnikov
2013-09-25 13:05               ` Ludovic Courtès
2013-09-26  2:47                 ` Nikita Karetnikov
2013-09-26  9:49                   ` Ludovic Courtès
2013-09-27 19:04                     ` Ludovic Courtès
2013-09-03 19:21   ` MIPS64/N64 support (was: Goals for 0.4) Nikita Karetnikov
2013-09-03 20:45     ` MIPS64/N64 support Ludovic Courtès
2013-09-04  0:35       ` Nikita Karetnikov
2013-09-04 12:18         ` Ludovic Courtès
2013-09-06  8:35           ` Nikita Karetnikov
2013-09-06  9:46             ` Ludovic Courtès
2013-09-07  2:45               ` Nikita Karetnikov
2013-09-07 12:57                 ` Ludovic Courtès
2013-09-08 14:21                   ` Nikita Karetnikov
2013-09-08 19:54                     ` Ludovic Courtès
2013-09-09  5:38                       ` Nikita Karetnikov
2013-09-09 16:47                         ` Ludovic Courtès
2013-09-27  2:16                           ` Nikita Karetnikov
2013-09-27 19:00                             ` Ludovic Courtès
2013-09-29 13:27                               ` Nikita Karetnikov
2013-09-29 13:31                                 ` Ludovic Courtès
2013-09-29 23:18                                   ` Nikita Karetnikov
2013-09-30 11:32                                     ` Nikita Karetnikov
2013-09-30 16:26                                       ` Ludovic Courtès
2013-09-30 21:51                                         ` Nikita Karetnikov
2013-10-01  7:09                                           ` Lluís Batlle i Rossell
2013-10-01  7:48                                             ` Nikita Karetnikov
2013-10-01  8:03                                               ` Lluís Batlle i Rossell
2013-10-01  8:55                                                 ` Nikita Karetnikov
2013-10-01  8:59                                                   ` Nikita Karetnikov
2013-10-01  9:30                                                   ` Lluís Batlle i Rossell
2013-10-01 10:06                                                     ` Nikita Karetnikov
2013-10-01 10:04                                                       ` Lluís Batlle i Rossell
2013-10-01 11:25                                                         ` Ludovic Courtès
2013-10-01 11:56                                                           ` Lluís Batlle i Rossell
2013-10-07 18:47                                       ` Mark H Weaver
2013-10-07 19:39                                         ` Ludovic Courtès
2013-10-08 23:03                                           ` Mark H Weaver
2013-10-09  6:53                                             ` Mark H Weaver
2013-10-09 10:42                                               ` Ludovic Courtès
2013-10-09 10:39                                             ` Ludovic Courtès
2013-10-10  4:08                                       ` Mark H Weaver
2013-09-30 16:09                                     ` Ludovic Courtès
2013-08-29 15:49 ` Goals for 0.4 Amirouche Boubekki
2013-08-29 20:04   ` Ludovic Courtès
2013-08-30 16:09     ` Cyprien Nicolas
2013-08-30 17:40       ` Amirouche Boubekki
2013-08-30 19:31       ` Overlays Ludovic Courtès
2013-08-30 20:42         ` Overlays Nikita Karetnikov
2013-08-30 21:21           ` Overlays Ludovic Courtès
2013-08-31 10:56             ` Overlays Amirouche Boubekki
2013-08-31 15:57               ` Overlays Ludovic Courtès
2013-08-29 20:42 ` Goals for 0.4 Andreas Enge
2013-08-29 21:32   ` Ludovic Courtès
2013-09-25  8:43   ` Andreas Enge
2013-09-25 13:13     ` Ludovic Courtès
2013-09-26 11:35       ` Andreas Enge
2013-09-28 13:25         ` Ludovic Courtès
2013-09-29 21:29           ` Alex Sassmannshausen
2013-09-02 17:33 ` Cyril Roelandt
2013-09-02 19:38   ` Ludovic Courtès
2013-09-02 19:40     ` Cyril Roelandt
2013-09-02 21:35       ` Ludovic Courtès
2013-09-06  9:19 ` ‘--no-substitutes’ is ignored on i686 (was: Goals for 0.4) Nikita Karetnikov
2013-09-06  9:59   ` ‘--no-substitutes’ is ignored on i686 Ludovic Courtès
2013-09-07  8:43     ` Nikita Karetnikov
2013-09-07 13:00       ` Ludovic Courtès
2013-09-08 11:53         ` Nikita Karetnikov
2013-09-08 11:51           ` Cyril Roelandt
2013-09-08 13:22             ` Nikita Karetnikov
2013-09-24 21:59 ` Goals for 0.4 Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878uyo60gp.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=guix-devel@gnu.org \
    --cc=nikita@karetnikov.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).