guix-devel
[Top][All Lists]
Advanced

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

Re: Display diffs between generations.


From: Ludovic Courtès
Subject: Re: Display diffs between generations.
Date: Mon, 29 Aug 2016 18:25:29 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Hi Roel,

I’ve just tried the patch and I think it’s awesome!  I’ve also always
been dissatisfied with what ‘--list-generations’ provides—it’s
inconvenient as soon as you have more than a handful packages.

Perhaps we could add an optional argument to --list-generations where:

  --list-generations

would be equivalent to:

  --list-generations=diff

and:

  --list-generations=full

would restore the previous behavior.  It doesn’t cost much to do that.

WDYT?

I have only minor stylistic comments:

Roel Janssen <address@hidden> skribis:

> +(define (display-profile-content-diff profile number)

Or just ‘display-profile-diff’?

> +  "Display the changed packages in PROFILE with generation specified by 
> NUMBER."

“… compared to generation NUMBER”?

> +  (define (equal-entry? first second)
> +    (string= (manifest-entry-item first)
> +             (manifest-entry-item second)))
> +
> +  (define* (display-entries entries #:optional (prefix " "))
> +    (for-each
> +     (match-lambda
> +       (($ <manifest-entry> name version output location _)
> +        (format #t "  ~a ~a\t~a\t~a\t~a~%"
> +                prefix name version output location)))
> +     entries))

In general, I find it clearer to define the singular form and inline the
‘for-each’:

  (define display-entry
    (match-lambda …))

  …

  (for-each display-entry entries)

Otherwise LGTM!

Thank you for addressing this!

Ludo’.



reply via email to

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