From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>,
Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/2] api: disks: directory: fail if mount unit already exists
Date: Wed, 27 Nov 2024 16:26:51 +0100 [thread overview]
Message-ID: <8e912dbf-8afe-4018-972f-18d6cc037833@proxmox.com> (raw)
In-Reply-To: <D5X2BRGHBE6W.3OM2GU7H6CRPN@proxmox.com>
Am 27.11.24 um 16:23 schrieb Shannon Sterz:
> On Wed Nov 27, 2024 at 4:06 PM CET, Fiona Ebner wrote:
>> + if std::path::PathBuf::from(&mount_unit_path).exists() {
>> + bail!("systemd mount unit {mount_unit_path:?} already exists");
>
> nit: relying on the `Debug` trait here to quote the string feels a bit..
> hacky to me? maybe just:
>
> ```rs
> bail!("systemd mount unit \"{mount_unit_path}\" already exists");
> ```
>
> or use single quotes as we do elsewhere.
>
FWIW, we (mis)use this semi-frequently already, it might not be the cleanest
thing but works out OK; I'm fine either way, just wanted to mention that if
this really has some issue, or is considered non-idiomatic, then a tree-wide
clean-up would be warranted, otherwise its usage will probably grow due to
copying existing code as base for new dev work.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-11-27 15:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-27 15:06 [pbs-devel] [PATCH proxmox-backup 0/2] " Fiona Ebner
2024-11-27 15:06 ` [pbs-devel] [PATCH proxmox-backup 1/2] api: disks: directory: factor out helper for mount unit path Fiona Ebner
2024-11-27 15:21 ` Shannon Sterz
2024-11-27 15:06 ` [pbs-devel] [PATCH proxmox-backup 2/2] api: disks: directory: fail if mount unit already exists Fiona Ebner
2024-11-27 15:23 ` Shannon Sterz
2024-11-27 15:26 ` Thomas Lamprecht [this message]
2024-11-27 16:08 ` Fiona Ebner
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=8e912dbf-8afe-4018-972f-18d6cc037833@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=s.sterz@proxmox.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.
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.