GNU bug report logs - #62102
[PATCH] services: Add whoogle-service-type.

Previous Next

Full log

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

From: Miguel Ángel Moreno <contact <at> conses.eu>
To: 62102 <at> debbugs.gnu.org
Subject: Re: bug#62102: [PATCH] services: Add whoogle-service-type.
Date: Fri, 09 Jun 2023 23:07:02 +0200

On 2023-06-09 23:03, Ludovic Courtès wrote:

> Hi conses,
>
> Did you have a chance to look into Bruno’s suggestions below?
>
> Please let us know what you think.
>

Hey Ludovic,

Yes, I've noted them but I haven't gotten around to implementing them
yet.  I'll give this a shot soon.

>
> Bruno Victal <mirai <at> makinata.eu> skribis:
>
>> Hi,
>>
>> On 2023-03-10 20:11, conses wrote:
>>>  
>>> +            whoogle-service-type
>>> +            whoogle-configuration
>>> +
>>
>> [...]
>>
>>>              patchwork-database-configuration
>>>              patchwork-database-configuration?
>>>              patchwork-database-configuration-engine
>>> @@ -1575,6 +1580,47 @@ (define varnish-service-type
>>>     (default-value
>>>       (varnish-configuration))))
>>>  
>>> +
>>
>> Missing page-break character here?
>> If you're using Emacs you can insert one with C-q C-l.
>>
>>> +;;;
>>> +;;; Whoogle
>>> +;;;
>>> +
>>> +(define-configuration/no-serialization whoogle-configuration
>>> +  (whoogle
>>> +    (package whoogle-search)
>>> +    "The @code{whoogle-search} package to use."))
>>
>> I prefer this field to be named 'package' instead, to make it less prone
>> to confusion. The accessor, whoogle-configuration-package, should be exported
>> as well.
>>
>>> +(define (whoogle-shepherd-service config)
>>> +  (list
>>> +   (shepherd-service
>>> +    (provision '(whoogle-search))
>>> +    (start #~(make-forkexec-constructor
>>> +              (list (string-append #$(whoogle-configuration-whoogle config)
>>> +                                   "/bin/whoogle-search"))
>>
>> In general, you may prefer to use match-record instead of using accessors
>> as it results in much less code to type.
>>
>>> +              #:environment-variables
>>> +              (append (list "CONFIG_VOLUME=/var/cache/whoogle-search")
>>> +                      (default-environment-variables))))
>>
>> Is (default-environment-variables) necessary?
>>
>>> +    (stop #~(make-kill-destructor))
>>> +    (documentation "Run a @code{whoogle-search} instance."))))
>>> +
>>> +(define (whoogle-profile-service config)
>>> +  (list
>>> +   (whoogle-configuration-whoogle config)))
>>
>> [...]
>>
>>> +
>>> +(define whoogle-service-type
>>> +  (service-type
>>> +   (name 'whoogle-search)
>>> +   (extensions
>>> +    (list
>>> +     (service-extension
>>> +      shepherd-root-service-type
>>> +      whoogle-shepherd-service)
>>> +     (service-extension
>>> +      profile-service-type
>>> +      whoogle-profile-service)))
>>
>> You can use match-record here or use the accessor here instead.
>> (e.g., (compose list whoogle-configuration-package))
>>
>>
>> Could you add a system test for this service as well?
>> It makes things easier to check and maintain.
>>
>>
>> Cheers,
>> Bruno

-- 
Best regards,
Miguel Ángel Moreno

Previous Next

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