Re: [pve-devel] [PATCH proxmox-ve-rs 1/1] partially fix #6176: config: guest: change default for firewall key

From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Maximiliano Sandoval <m.sandoval@proxmox.com>
Subject: Re: [pve-devel] [PATCH proxmox-ve-rs 1/1] partially fix #6176: config: guest: change default for firewall key
Date: Wed, 19 Feb 2025 15:25:33 +0100	[thread overview]
Message-ID: <72f66ab3-857c-466a-b73e-fa881195de7c@proxmox.com> (raw)
In-Reply-To: <s8o34gahz7u.fsf@proxmox.com>

On 2/19/25 13:52, Maximiliano Sandoval wrote:
>> -#[derive(Debug)]
>> +#[derive(Debug, Deserialize)]
>>  #[cfg_attr(test, derive(Eq, PartialEq))]
>> -pub struct NetworkDevice {
>> +pub struct QemuNetworkDevice {
>>      model: NetworkDeviceModel,
>> +    #[serde(rename = "macaddr")]
>> +    mac_address: MacAddress,
>> +    firewall: Option<bool>,
> 
> Why not use `#[serde(default)]` and always get a boolean? The only place
> it is used uses .unwrap_or(false) (unwrap_or_default would be preferable
> imho).

The reasoning for this is that it is modeled 1:1 after what's in the
property string. It works when reading, but when trying to serialize the
struct you then always have to provide the value for firewall and cannot
omit it.

unwrap_or_default() is fine by me, i thought unwrap_or(false) makes it
clearer but I don't mind at all.

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel