Hi Maxim,

I just sent a v3 of the patch series.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi,
>
> Roman Scherer <roman@burningswell.com> writes:
>
>> * gnu/services/sound.scm (speakersafetyd): Add log file.
>> * doc/guix.texi: Document log-file field.
>>
>> Change-Id: I870bc7bfd69249da3a9c981f627e751395386bd2
>> ---
>>  doc/guix.texi          |  3 +++
>>  gnu/services/sound.scm | 10 +++++++---
>>  2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index a0f2a83c36..9a6084e994 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -27271,6 +27271,9 @@ Sound Services
>>  @item @code{group} (default: @code{"speakersafetyd"}) (type: string)
>>  The group to run the Speaker Safety Daemon as.
>>
>> +@item @code{log-file} (default: @code{"/var/log/speakersafetyd.log"}) (type: string)
>> +The path to the Speaker Safety Daemon log file.
>> +
>>  @item @code{maximum-gain-reduction} (default: @code{7}) (type: integer)
>>  Maximum gain reduction before panicking, useful for debugging.
>>
>> diff --git a/gnu/services/sound.scm b/gnu/services/sound.scm
>> index e5c26e2495..39b5d043a3 100644
>> --- a/gnu/services/sound.scm
>> +++ b/gnu/services/sound.scm
>> @@ -293,6 +293,9 @@ (define-configuration/no-serialization speakersafetyd-configuration
>>    (group
>>     (string "speakersafetyd")
>>     "The group to run the Speaker Safety Daemon as.")
>> +  (log-file
>> +   (string "/var/log/speakersafetyd.log")
>> +   "The path to the Speaker Safety Daemon log file.")
>
> The convention in GNU is to use 'path' only for search paths; the
> preferred term for file names is 'file name'.
>

I changed it.

>>    (maximum-gain-reduction
>>     (integer 7)
>>     "Maximum gain reduction before panicking, useful for debugging.")
>> @@ -305,7 +308,7 @@ (define-configuration/no-serialization speakersafetyd-configuration
>>
>>  (define speakersafetyd-accounts
>>    (match-record-lambda <speakersafetyd-configuration>
>> -      ( blackbox-directory configuration-directory group
>> +      ( blackbox-directory configuration-directory group log-file
>>          maximum-gain-reduction speakersafetyd user)
>>      (list (user-group
>>             (name group)
>> @@ -320,7 +323,7 @@ (define speakersafetyd-accounts
>>
>>  (define speakersafetyd-activation
>>    (match-record-lambda <speakersafetyd-configuration>
>> -      ( blackbox-directory configuration-directory group
>> +      ( blackbox-directory configuration-directory group log-file
>>          maximum-gain-reduction speakersafetyd user)
>>      (with-imported-modules (source-module-closure '((gnu build activation)))
>>        #~(begin
>> @@ -334,7 +337,7 @@ (define speakersafetyd-activation
>>
>>  (define speakersafetyd-shepherd-service
>>    (match-record-lambda <speakersafetyd-configuration>
>> -      ( blackbox-directory configuration-directory group
>> +      ( blackbox-directory configuration-directory group log-file
>>          maximum-gain-reduction speakersafetyd user)
>
> As mentioned earlier, make sure to expose only the fields needed in the
> above match-record-lambda forms.
>

Nice! I somehow thought I had to list all of them, and even in the right
order :)

>>      (shepherd-service
>>       (documentation "Run the speaker safety daemon")
>> @@ -346,6 +349,7 @@ (define speakersafetyd-shepherd-service
>>                       "--blackbox-path" #$blackbox-directory
>>                       "--max-reduction" (number->string #$maximum-gain-reduction))
>>                 #:group #$group
>> +               #:log-file #$log-file
>>                 #:supplementary-groups '("audio")
>>                 #:user #$user))
>>       (stop #~(make-kill-destructor)))))
>
> Otherwise, LGTM!
>
> Could you send a hopefully final v3?

Thanks for your review. Could you have another look?

Roman.