all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Tanguy LE CARROUR <tanguy@bioneland.org>
To: Bruno Victal <mirai@makinata.eu>
Cc: 62969@debbugs.gnu.org
Subject: [bug#62969] [PATCH] home: Add msmtp service.
Date: Sun, 23 Apr 2023 19:10:15 +0200	[thread overview]
Message-ID: <168226981597.13515.12105199384744750840@localhost> (raw)
In-Reply-To: <4f8624a0-31fe-1ea5-733a-b2e1917291b4@makinata.eu>

Hi Bruno,

Thank you so much for you comments!
I've applied (almost, see below) all of your suggestions and I will send a v2.


Quoting Bruno Victal (2023-04-20 18:36:59)
> On 2023-04-20 15:42, Tanguy Le Carrour wrote:
> > +(define (configuration-serialize-maybe-string field-name value)
> > +  #~(if #$(maybe-value-set? value)
> > +      (string-append #$(uglify-symbol field-name) " " #$value "\n")
> > +      ""))> +
> > +(define (configuration-serialize-maybe-integer field-name value)
> > +  #~(if #$(maybe-value-set? value)
> > +      (string-append #$(uglify-symbol field-name) " " (number->string #$value) "\n")
> > +      ""))
> > +
> > +(define (configuration-serialize-maybe-boolean field-name value)
> > +  #~(if #$(maybe-value-set? value)
> > +      (string-append #$(uglify-symbol field-name) " " (if #$value "on" "off") "\n")
> > +      ""))
> You don't have to perform the maybe-value-set? checks, it is automatically done for you.
> The only cases where this isn't true is if you explicitly override the serializer in
> define-configuration. [2]

This is the only thing I can't figure out how to make work?!
When I remove the `(if #$(maybe-value-set? value) …)` I get the
following error:

```
Backtrace:
          10 (primitive-load "/gnu/store/ajcf46q8yr9sb9n90psa5ay96jw?")
In ice-9/ports.scm:
   433:17  9 (call-with-output-file _ _ #:binary _ #:encoding _)
In ice-9/eval.scm:
    159:9  8 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
    155:9  7 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
   173:39  6 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
    159:9  5 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
   173:55  4 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
   173:55  3 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
   279:15  2 (_ #(#(#<directory (guile-user) 7ffff3fd7c80>) #<outp?>))
   223:20  1 (proc #(#(#<directory (guile-user) 7ffff3fd7c80>) #<o?>))
In unknown file:
           0 (%resolve-variable (7 . %unset-marker%) #<directory (gu?>)

ERROR: In procedure %resolve-variable:
Unbound variable: %unset-marker%
```


> > +  (prefix configuration-))
> These are poor prefix choices for a module named (gnu home services mail).
> If it were (gnu home services msmtp) or (gnu home services mail msmtp) it would be acceptable
> but a module named (gnu home services mail) is expected to eventually contain multiple services that
> have to co-exist. These prefixes are too non-specific and confusion prone for such circumstances.
> 
> I'd set this to (prefix msmtp-configuration-). (the same logic applies to the remaining (prefix ...) lines)

I totally agree! You guessed correctly, this home service was living
inside `(bioneland home services msmtp)`, so the naming seemed to make
sense at the time, but not any more!

Thanks again for your time and help!

-- 
Tanguy




  reply	other threads:[~2023-04-23 17:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20 14:42 [bug#62969] [PATCH] home: Add msmtp service Tanguy Le Carrour
2023-04-20 16:36 ` Bruno Victal
2023-04-23 17:10   ` Tanguy LE CARROUR [this message]
2023-04-23 17:12 ` [bug#62969] [PATCH v2] " Tanguy Le Carrour
2023-04-30 21:30   ` [bug#62969] [PATCH] " Ludovic Courtès
2023-05-03 10:01     ` [bug#62969] [PATCH v3] " Tanguy LE CARROUR
2023-05-03 20:27       ` Ludovic Courtès
2023-05-17  8:52         ` Tanguy LE CARROUR
2023-05-17  8:51 ` [bug#62969] [PATCH v4] " Tanguy Le Carrour
2023-05-29 21:43   ` bug#62969: [PATCH] " Ludovic Courtès
2023-05-30  6:54     ` [bug#62969] " Tanguy LE CARROUR

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=168226981597.13515.12105199384744750840@localhost \
    --to=tanguy@bioneland.org \
    --cc=62969@debbugs.gnu.org \
    --cc=mirai@makinata.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.