GNU bug report logs - #74542
[PATCH 00/11] Improved tooling for package updates

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Tue, 26 Nov 2024 10:33:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, 74542 <at> debbugs.gnu.org
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, Ludovic Court?s <ludo <at> gnu.org>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Christopher Baines <guix <at> cbaines.net>
Subject: [bug#74542] [PATCH 06/11] guix build: Add ‘--dependents’.
Date: Wed, 27 Nov 2024 20:12:27 +0100
Hi,

On Tue, 26 Nov 2024 at 11:33, Ludovic Courtès <ludo <at> gnu.org> wrote:

> +@item --dependents[=@var{depth}]
> +@itemx -T [@var{depth}]

Cosmetic: argh, ’-M’ is already taken under “guix build”! Because it
would be nice to have something similar as with “guix graph
--max-depth”.  Yes, both depths does not have the same meaning but
that’s still a ’max depth’ for both cases. ;-)

Well…

> +The list of dependents is computed in the same way as with @command{guix
> +refresh --list-dependent} (@pxref{Invoking guix refresh}).

… “guix refresh” already uses ’-T’ but for another meaning
(--list-transitive).  Therefore, I think something other than ’-T’
should be better.  Maybe ’-D’ since it cannot conflict with “guix
shell”..

> +  -T, --dependents[=N]   build dependents of the following package, up to
> +                         depth N"))

As said, ’-T’ might be confusing with “guix refresh -T” especially when
’dependents’ is more or less connected with “guix refresh -l”.

> +         (option '(#\T "dependents") #f #t
> +                 (lambda (opt name arg result)
> +                   (alist-cons 'dependents
> +                               (or (and=> arg string->number*) +inf.0)
> +                               result)))

Idem.

> +(define (options->things-to-build store opts)

[...]

> -    (map transform (options->things-to-build opts)))
> +    (map transform (options->things-to-build store opts)))

I thought ’store’ could have been hidden, but not.  Since it’s already
passed to ’options->derivations’. :-)


Well, aside the cosmetic, LGTM.

Cheers,
simon




This bug report was last modified 169 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.