Re: [pbs-devel] [PATCH proxmox-backup 4/7] api: admin: trigger sync jobs only on datastore mount

[Christian Ebner <c.ebner@proxmox.com>]

On 5/15/25 13:33, Hannes Laimer wrote:
> 
> 
> On 2/4/25 15:34, Christian Ebner wrote:
>> This patch should rather be placed as preparatory patch before patch 3 
>> and changes to `do_sync_jobs` directly included in the previous patch.
>>
> 
> Maybe, but these changes only really make sense in this context. If done
> before it would be something like
> ```
>            move |_worker| async move {
> -            do_mount_device(datastore.clone())?;
> +            if !do_mount_device(datastore.clone())? {
> +                return Ok(());
> +            }
>        }
> ```
> I feel like having this change in the context of where it is needed does
> add some value. Especially when one tries to find out later why this was
> needed. Having code under the `return Ok(())` is pretty clear in terms
> of what it is we try to skip here in some cases. Without I could imagine
> being like "th was that good for?", then having to go through the commit
> message. IMOH, having the extra context here is better than a possibly
> better order. But no hard feelings though, if you'd really like this one
> before I'll move it for v2.

No, fine by me to keep as is given your reasoning!


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