all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Mikhail Tsykalov <tsymsh@gmail.com>
Cc: 41143@debbugs.gnu.org
Subject: [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings.
Date: Sun, 04 Oct 2020 12:28:32 +0200	[thread overview]
Message-ID: <874knaffov.fsf@gnu.org> (raw)
In-Reply-To: <20201001224900.28989-1-tsymsh@gmail.com> (Mikhail Tsykalov's message of "Fri, 2 Oct 2020 01:48:59 +0300")

Hi Mikhail,

Mikhail Tsykalov <tsymsh@gmail.com> skribis:

> * gnu/system/mapped-devices.scm (<mapped-device>): Rename constructor to
> %mapped-device.
> [target]: Remove field.
> [targets]: New field. Adjust users.
> (mapped-device-compatibility-helper, mapped-device): New macros.
> (mapped-device-target): New deprecated procedure.

Thanks for following up.  I think we’re almost done, some comments
below:

> --- a/gnu/system/linux-initrd.scm
> +++ b/gnu/system/linux-initrd.scm
> @@ -196,7 +196,7 @@ upon error."
>      ;; List of gexps to open the mapped devices.
>      (map (lambda (md)
>             (let* ((source (mapped-device-source md))
> -                  (target (mapped-device-target md))
> +                  (target (mapped-device-targets md))

I think we should write ‘targets’ (plural) everywhere.  That can help
avoid confusion IMO.

> -                        #$target)))))
> +                        #$(car target))))))
>  
>  (define (close-luks-device source target)
>    "Return a gexp that closes TARGET, a LUKS device."
>    #~(zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
> -                    "close" #$target)))
> +                    "close" #$(car target))))

As per our coding convention (info "(guix) Data Types and Pattern
Matching"), I’d recommend using ‘match’

  (define (close-luks-device source targets)
    (match targets
      ((target)
       #~(zero? (system* … #$target)))))

That has the added benefit that it errors out if TARGETS is not exactly
a one-element list.

>  (define (close-raid-device sources target)
>    "Return a gexp that stops the RAID device TARGET."
>    #~(zero? (system* #$(file-append mdadm-static "/sbin/mdadm")
> -                    "--stop" #$target)))
> +                    "--stop" #$(car target))))

Same here.

Could you also update “Mapped Devices” in doc/guix.texi to mention the
new ‘targets’ field?

Thanks,
Ludo’.




  parent reply	other threads:[~2020-10-04 10:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-09  1:12 [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings tsmish
2020-05-09  1:22 ` [bug#41143] [PATCH 2/2] mapped-devices: Add 'lvm-device-mapping' tsmish
2020-09-09 20:48   ` Ludovic Courtès
2020-05-14 22:53 ` [bug#41143] Some clarification Mikhail Tsykalov
2020-05-15  1:17 ` [bug#41143] [PATCH] mapped-devices: Document lvm-mapping-device Mikhail Tsykalov
2020-06-06 13:40 ` [bug#41143] [PATCH 1/2] mapped-devices: Allow target to be list of strings Lars-Dominik Braun
2020-06-06 20:16   ` Mikhail Tsykalov
2020-06-07  6:48     ` Lars-Dominik Braun
2020-09-09 20:38 ` Ludovic Courtès
2020-09-24 16:09   ` Mikhail Tsykalov
2020-09-25  9:34     ` Ludovic Courtès
2020-09-25 13:36       ` Mikhail Tsykalov
2020-09-25 16:20         ` Ludovic Courtès
2020-10-01 22:48           ` [bug#41143] [PATCH v2 " Mikhail Tsykalov
2020-10-01 22:49             ` [bug#41143] [PATCH v2 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-10-04 10:34               ` Ludovic Courtès
2020-10-04 10:28             ` Ludovic Courtès [this message]
2020-11-05  9:48               ` [bug#41143] [PATCH v2 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-11-06  9:47                 ` [bug#41143] [PATCH v3 " Mikhail Tsykalov
2020-11-06  9:47                   ` [bug#41143] [PATCH v3 2/2] mapped-devices: Add 'lvm-device-mapping' Mikhail Tsykalov
2020-11-25 23:09                   ` bug#41143: [PATCH v3 1/2] mapped-devices: Allow target to be list of strings Ludovic Courtès
2020-10-01 23:15           ` [bug#41143] [PATCH " Mikhail Tsykalov

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=874knaffov.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=41143@debbugs.gnu.org \
    --cc=tsymsh@gmail.com \
    /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.