Re: [pbs-devel] [PATCH proxmox-backup v2] client/pull: log snapshots that are skipped because of time

[Dietmar Maurer <dietmar@proxmox.com>]

> +impl std::fmt::Display for SkipInfo {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        if self.count > 1 {
> +            write!(
> +                f,
> +                "{} snapshots ({}..{}) that are older than the newest local snapshot",
> +                self.count,
> +                proxmox::tools::time::epoch_to_rfc3339_utc(self.oldest)
> +                    .map_err(|_| std::fmt::Error)?,
> +                proxmox::tools::time::epoch_to_rfc3339_utc(self.newest)
> +                    .map_err(|_| std::fmt::Error)?,
> +            )

what is the purpose of this complex message (why we want to show self.oldest and self.newest)?
Its confusing me more than it helps...

> +        } else if self.count == 1 {
> +            write!(
> +                f,
> +                "1 snapshot ({}) that is older than the newest local snapshot",
> +                proxmox::tools::time::epoch_to_rfc3339_utc(self.oldest)
> +                    .map_err(|_| std::fmt::Error)?,
> +            )

do we really need this special case?

> +        } else {
> +            write!(f, "0 snapshots")

Instead, I would avoid to call this function if count is 0 ...

> +        }
> +    }
> +}
> +
>  pub async fn pull_group(