Re: [pve-devel] [PATCH v1 installer 08/18] auto-installer: add answer file definition

[Christoph Heiss <c.heiss@proxmox.com>]

Some comments inline, more-or-less just a (clippy) nit.

On Tue, Jan 23, 2024 at 06:00:43PM +0100, Aaron Lauterer wrote:
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  proxmox-auto-installer/src/answer.rs | 147 +++++++++++++++++++++++++++
>  proxmox-auto-installer/src/lib.rs    |   1 +
>  2 files changed, 148 insertions(+)
>  create mode 100644 proxmox-auto-installer/src/answer.rs
>
> diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
> new file mode 100644
> index 0000000..0f6c593
> --- /dev/null
> +++ b/proxmox-auto-installer/src/answer.rs
> @@ -0,0 +1,147 @@
[..]
> +
> +impl ZfsOptions {
> +    pub fn new() -> ZfsOptions {
> +        ZfsOptions {
> +            ashift: None,
> +            arc_max: None,
> +            checksum: None,
> +            compress: None,
> +            copies: None,
> +            hdsize: None,
> +        }
> +    }
> +}
Can be replaced by auto-deriving `Default` and then using `::default()`.
Would also save a quite a few lines.

Clippy screams a bit about it anyway:

warning: you should consider adding a `Default` implementation for `ZfsOptions`
  --> proxmox-auto-installer/src/answer.rs:82:5
   ...
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
   = note: `#[warn(clippy::new_without_default)]` on by default

[..]
> +
> +impl LvmOptions {
> +    pub fn new() -> LvmOptions {
> +        LvmOptions {
> +            hdsize: None,
> +            swapsize: None,
> +            maxroot: None,
> +            maxvz: None,
> +            minfree: None,
> +        }
> +    }
> +}
^ Same here about auto-deriving `Default`.

> +
> +#[derive(Clone, Deserialize, Serialize, Debug)]
> +pub struct BtrfsOptions {
> +    pub hdsize: Option<f64>,
> +}
> +
> +impl BtrfsOptions {
> +    pub fn new() -> BtrfsOptions {
> +        BtrfsOptions { hdsize: None }
> +    }
> +}
^ Same here about auto-deriving `Default`.

> diff --git a/proxmox-auto-installer/src/lib.rs b/proxmox-auto-installer/src/lib.rs
> index e69de29..7813b98 100644
> --- a/proxmox-auto-installer/src/lib.rs
> +++ b/proxmox-auto-installer/src/lib.rs
> @@ -0,0 +1 @@
> +pub mod answer;
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>