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

Previous Next

Full log

Message #98 received at 74542 <at> debbugs.gnu.org (full text, mbox):

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: Re: [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

Previous Next

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