Subject: Re: [PATCH v3] home: Add msmtp service.

From: Ludovic Courtès <[email-masked]>
Date: Wed, 03 May 2023 22:27:03 +0200

Hello!

Tanguy LE CARROUR <[email-masked]> skribis:

> Quoting Ludovic Courtès (2023-04-30 23:30:10)
>> Tanguy Le Carrour <[email-masked]> skribis:
>> 
>> > * gnu/home/services/mail.scm: New file.
>> > * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
>> > * doc/guix.texi (Mailing): New node.
>> 
>> I get:
>> 
>> --8<---------------cut here---------------start------------->8---
>> [ 94%] GUILEC   gnu/home/services/mail.go
>> gnu/home/services/mail.scm:52:0: warning: possibly unbound variable `serialize-string'
>> gnu/home/services/mail.scm:53:0: warning: possibly unbound variable `serialize-boolean'
>> gnu/home/services/mail.scm:54:0: warning: possibly unbound variable `serialize-integer'
>> --8<---------------cut here---------------end--------------->8---
>>
>> I guess these procedures should be provided as well?
>
> I also get them when making `gnu/home/services/mail.go`. But aren't
> those "just" warnings.

It means that there’s code referring to these procedures, and that they
don’t exist.  It’s worth investigating anyway.

> Everything seems to work as expected and when I `guix home container
> test.home.scm` I get the expected `.config/msmtp/config` file.

Perhaps that doesn’t exercise those bits, maybe because you’re not using
string-valued or boolean-valued fields or something?

A couple of minor things:

> * gnu/home/services/mail.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * doc/guix.texi (Mailing): New node.
> +@subsection Mail Home Services
> +
> +The @code{(gnu home services mail)} modules provides services that help

“module” (singular)

> +@uref{https://marlam.de/msmtp, MSMTP} is a @acronym{SMTP, Simple Mail
> +Transfer Protocol} client.  It sends mails to a predefined SMTP server

“mail” (singular)

> +@code{~/.config/msmtp/config} file.

“@file”

This new intro + example look nice to me!

> +@item @code{logfile} (type: maybe-string)

[...]

> +@item @code{passwordeval} (type: maybe-string)
> +Set the password for authentication to the output (stdout) of the
> +command cmd.

Sorry for not noticing earlier but I’d suggest ‘log-file’ and
‘password-evaluation’ (?) to stick with the established naming
convention.

So I think beyond this the main issue is figuring out the missing
serialization procedures, and then we’re done!

Thanks,
Ludo’.