Subject: Re: bug#65732: [PATCH 0/2] Add the Build Farm Front-End

From: Christopher Baines <[email-masked]>
Date: Fri, 15 Sep 2023 10:09:18 +0100

Ludovic Courtès <[email-masked]> writes:

> Christopher Baines <[email-masked]> skribis:
>
>> This is intended to replace the functionality of the Guix Build Coordinator
>> queue builds script, and also provide a web interface for build farms.
>>
>> * gnu/services/guix.scm (<bffe-configuration>): New record type.
>> (bffe-configuration, bffe-configuration?,
>> bffe-configuration-package,
>> bffe-configuration-user,
>> bffe-configuration-group,
>> bffe-configuration-arguments
>> bffe-configuration-extra-environment-variables): New procedures.
>> (bffe-service-type): New variable.
>> * gnu/tests/guix.scm (%test-bffe): New variable.
>> * doc/guix.texi (Guix Services): Document the new service.
>
> Overall LGTM.
>
> Nitpick: you can make lines a bit longer, for instance:
>
>> +(define (bffe-activation config)
>> +  #~(begin
>> +      (use-modules (guix build utils))
>> +
>> +      (define %user
>> +        (getpw #$(bffe-configuration-user
>> +                  config)))
>
> ‘config’ can be moved to the previous line.
>
>> +      (chown "/var/run/bffe"
>> +             (passwd:uid %user)
>> +             (passwd:gid %user))))
>
> This can be a single line.
>
>> +   (extensions
>> +    (list
>> +     (service-extension shepherd-root-service-type
>> +                        bffe-shepherd-services)
>
> And: (list (service-extension …

I've tweaked the formatting as suggested above and pushed this to master
as 82abf6ddadc6139148660440a064e60ae68f238e.

Thanks,

Chris