guix-devel
[Top][All Lists]
Advanced

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

Re: branch master updated: gnu: Add passff.


From: Clément Lassieur
Subject: Re: branch master updated: gnu: Add passff.
Date: Tue, 31 Oct 2023 20:33:35 +0100
User-agent: Gnus/5.13 (Gnus v5.13)

Hi John,

Thanks for taking the time to write this message.

On Tue, Oct 31 2023, John Kehayias wrote:

> Right, it uses gexps but I think here the better and more explicit
> style would be to use inputs/native-inputs. Then instead of
> referencing directly like #$<package-variable-name> use
> #$(this-package-input "package-name") to get the store path. This I
> think is clearer and I believe better allows for inheritance,
> input-rewriting, and so on.

You are right, I'll amend my patch, taking this into account.

I believe `guix lint` should say something about it though, maybe warn
when using an input that is not explicitly added in the `input` field.
Those errors are hard to catch and are in our code base at several
places already.

I've also noticed there is some subtle in-between style in our code
base: people adding x as input but still using #$x in the package
definition instead of using #$(this-package-input "x").  It's wrong
because in this case input rewriting does not work.

G-Expressions allow for weird things.  Here's another weird pattern from
our code base: going from (inputs `("foo", ,(origin ...))) to #$(origin
...)  without inputs.  This removes input rewriting too.

> Feel free for anyone else to chime in on this point, I'm always
> looking to learn to improve my own packaging and review, but this is
> what I understand is preferred when possible.
>
>>> Was this change sent as a patch to guix-patches?
>>
>> No it wasn't.
>
> The mantra I've heard, and agree with, is that the
> trivial-build-system is anything but trivial. Not saying it wasn't the
> best choice here, or has anything to do with the above points, but
> thought it worth mentioning for anyone else.

Agreed.

> But this is also why I think it would have been better to have it go
> through review. I see there's been several followup commits to improve
> the style and fix things which also could have been avoided. Not a
> huge deal perhaps, but I would err on the side of review for something
> like this.

I thought my patch was trivial, but I was obviously wrong since I
(indeed) amended it twice, and now a third time.

> Of course, thanks for the contribution!

Thanks for the kind words :)

Clément



reply via email to

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