Re: [pve-devel] [PATCH proxmox-ve-rs 03/11] add intermediate fabric representation

[Gabriel Goller <g.goller@proxmox.com>]

On 28.02.2025 14:57, Thomas Lamprecht wrote:
>Am 14.02.25 um 14:39 schrieb Gabriel Goller:
>> This adds the intermediate, type-checked fabrics config. This one is
>> parsed from the SectionConfig and can be converted into the
>> Frr-Representation.
>
>The short description of the patch is good, but I would like to see more
>rationale here about choosing this way, like benefits and trade-offs to other
>options that got evaluated, if this can/will be generic for all fabrics planned,
>..., and definitively some more rust-documentation for public types and modules.

Yep, wrote together a small reasoning and will write some more
documentation for public types.

>One thing I noticed below, I did not managed to do a thorough review besides
>of that yet though.
>> [snip]
>> +impl Hostname {
>> +    pub fn new(name: impl Into<String>) -> Hostname {
>> +        Self(name.into())
>> +    }
>> +}
>> +
>> +// parses a bool from a string OR bool
>> +pub mod serde_option_bool {
>
>might be maybe something to put in proxmox-serde?

Yep, I agree.

Thanks for the review!



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